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: show boundary only with editable element focus #15466

Merged
merged 2 commits into from May 14, 2019

Conversation

Projects
None yet
3 participants
@ellatrix
Copy link
Member

commented May 6, 2019

Description

Fixes #15463. The focus check should be more specific to editable elements. If a parent element has focus, e.g. the block or body element, the boundary should not be visible.

Of course the boundary attribute should be removed on blur, but we're not ready yet to remove "lingering" selection state on blur (e.g. formatting buttons would not work).

How has this been tested?

See #15463.

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.
@aduth
Copy link
Member

left a comment

In my testing, this seems to work a little too well, i.e. I can't get the inline boundary to show again when clicking back inside the element.

highlight-2

Generally I'm not clear why either the style can't be assigned directly (related to #13838?) or the style selector be more specific to guarantee that we're targeting the intended element (is data-rich-text-format-boundary removed when changing a selection to guarantee we can't have two highlighted in the same field?).

@aduth

This comment has been minimized.

Copy link
Member

commented May 6, 2019

Oddly, in the prior comment, the caret is within the bolded text, but it is not marked up as data-rich-text-format-boundary:

image

@ellatrix

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

@aduth Yes, something else seems to be going wrong there. :/ I'll need to have a look. Also, when you use the arrow keys inside formatting, the boundary disappears.

@ellatrix ellatrix referenced this pull request May 10, 2019

Merged

RichText: fix RTL keyboard interactions #15496

5 of 5 tasks complete

@ellatrix ellatrix force-pushed the fix/boundary-focus branch from 5fb4ad5 to 32436bc May 14, 2019

Issue resolved with #15496.

@ellatrix ellatrix requested a review from aduth May 14, 2019

@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 14, 2019

@aduth

aduth approved these changes May 14, 2019

Copy link
Member

left a comment

Wonder if it's the sort of thing we could have end-to-end tests for? I'm okay as a follow-up task.

Otherwise, it's working well for me 👍

@youknowriad youknowriad merged commit f9151c8 into master May 14, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@youknowriad youknowriad deleted the fix/boundary-focus branch May 14, 2019

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.