Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tiled galleries: Disable captions #29776

Merged
merged 2 commits into from
Jan 3, 2019
Merged

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Dec 28, 2018

Captions in Tiled Galleries closely match core gallery captions. They are functional, but present many challenges such as #29706

Disable captions for now by commenting them out.

Via p1545949443175900-slack-jetpack-gutenberg

Changes proposed in this Pull Request

  • Disable captions

Testing instructions

  • Verify that captions are not rendered.
  • Verify that you can create a block from this branch, then load it with no parse/validation errors on a branch with captions (current master).

@sirreal sirreal self-assigned this Dec 28, 2018
@matticbot
Copy link
Contributor

@sirreal sirreal added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Block] Tiled Gallery labels Dec 28, 2018
@sirreal
Copy link
Member Author

sirreal commented Dec 28, 2018

Not that this invalidates existing blocks with captions. I do not expect restoring captions in the future to invalidate posts without captions.

@simison
Copy link
Member

simison commented Dec 28, 2018

More captionSelected code to be commented out here:

if ( this.state.captionSelected ) {
this.setState( {
captionSelected: false,
} );
}

Copy link
Member

@simison simison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than one piece of code that needs to be commented (and rebase obviously) this worked well.

Captions are a pretty essential feature so they'll need to be high on priority right after layouts.

Some issues with captions listed here and also outlined a way to bring them back but simplified; https://github.com/Automattic/wp-calypso/issues/29778

@MichaelArestad
Copy link
Contributor

This works for me

@ockham
Copy link
Contributor

ockham commented Jan 3, 2019

Going to rebase

@ockham ockham force-pushed the remove/g7g-tg-disable-caption branch from c9255b6 to 4365ea7 Compare January 3, 2019 14:08
@ockham
Copy link
Contributor

ockham commented Jan 3, 2019

More captionSelected code to be commented out here:

if ( this.state.captionSelected ) {
this.setState( {
captionSelected: false,
} );
}

2053b47

Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as described, and code looks sane. :shipit:

@ockham
Copy link
Contributor

ockham commented Jan 3, 2019

I'll merge on behalf of @sirreal and @simison

@ockham ockham merged commit 12c38a5 into master Jan 3, 2019
@ockham ockham deleted the remove/g7g-tg-disable-caption branch January 3, 2019 14:23
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 3, 2019
blowery added a commit that referenced this pull request Jan 3, 2019
…sh-2019

* origin/master:
  Disable select when only one domain exiists (#29873)
  Tiled galleries: Disable captions (#29776)
  Prevent scrolling up when opening a dialog (#29832)
  Gutenberg: Move Shortlinks to production (#29883)
  Gutenlypso: add convert to blocks dialog (#29790)
  Gutenberg: Move Related Posts to production blocks (#29650)
  Analytics: fix ad-tracking issue on signup for non-gdpr countries (#29741)
  Update mobile phone validation module (used for 2fa) (#29740)
  Gutenlypso: make sure titles load on second edit (#29877)
  Site Picker: Change wording of /page and /block-editor to match /post (#29859)
  Gutenberg: update copy link in page list to be editor aware
  Gutenberg: use core approach of initialEdits over overridePost
  Gutenberg: when duplicating a post, override post content
  Gutenberg: update duplicate url when Gutenlypso is enabled
  Refactor: Replace use of key-mirror with inline code (#29857)
  Fixes wrong selected domain name (#29824)
  Turn off prettier for SASS, use stylelint instead (#29697)
  Gutenberg: Add copy button to Shortlinks (#29556)
  add a section name to the body class (#29563)
@sirreal
Copy link
Member Author

sirreal commented Jan 7, 2019

Thanks for bringing this home, folks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants