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

RichText: fix isSelected #17451

Merged
merged 1 commit into from Sep 16, 2019

Conversation

@ellatrix
Copy link
Member

commented Sep 16, 2019

Description

Attempt to fix #17405. Looks like the isSelected prop should only be looked at if the current block is selected. Currently we assume that the block manages that, but in previous versions there was a check internally if the wrapping block is selected.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Sep 16, 2019

@afercia Would be great to know if this fixes your issue. #17405 (comment)

@youknowriad

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

I tested this PR in addition with #17450 and the Yoast blocks seem to work properly. I also did a round of test for other blocks, everything seems to work as intended.

I'm approving both PRs. It would be good to try to add an e2e test for this.

@youknowriad youknowriad merged commit 61f4a25 into master Sep 16, 2019
4 of 7 checks passed
4 of 7 checks passed
pull-request-automation
Details
Header rules - gutenberg-playground No header rules processed
Details
Pages changed - gutenberg-playground 3 new files uploaded
Details
Redirect rules - gutenberg-playground No redirect rules processed
Details
Mixed content - gutenberg-playground No mixed content detected
Details
Travis CI - Pull Request Build Passed
Details
netlify/gutenberg-playground/deploy-preview Deploy preview ready!
Details
@youknowriad youknowriad deleted the try/is-selected-fix branch Sep 16, 2019
@youknowriad youknowriad added this to the Gutenberg 6.5 milestone Sep 16, 2019
@ellatrix

This comment has been minimized.

Copy link
Member Author

commented Sep 16, 2019

Yes, I agree. We specifically need an implementation RichText in a block where the block doesn't remove rich text selection state based on block selection.

@afercia

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

Thanks! Seems to work for me. Was waiting for my colleagues to double check :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.