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.

Sign up for GitHub

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

Jump to bottom

Try fixing the tab navigation issue #12390

Merged
merged 2 commits into from
Nov 28, 2018
Merged

Try fixing the tab navigation issue #12390

merged 2 commits into from
Nov 28, 2018

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Nov 28, 2018 •
edited
Loading

Rerendring RichText components was stealing the focus causing a tab navigation issue.

Fixes #11773.

@ellatrix
Copy link
Member

This will break:

merge

@ellatrix
Copy link
Member

I'm fine figuring out another way to set the caret at the end.

ellatrix
ellatrix approved these changes Nov 28, 2018
View reviewed changes
Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Looks good!

@youknowriad youknowriad added this to the 4.6 milestone Nov 28, 2018
@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Nov 28, 2018
@youknowriad youknowriad merged commit b5d4e15 into master Nov 28, 2018
@youknowriad youknowriad deleted the try/fix-tab-navigation branch November 28, 2018 18:17
@tofumatt
Copy link
Member

tofumatt commented Nov 28, 2018 •
edited
Loading

Thanks a million for getting to this while I stepped away from my Mac for a few hours! 😅

I would certainly like to see this in the WordPress 5.0 release, are others cool with seeing it in? I don't think it's mandatory (I didn't consider it a blocker per se) but it's certainly a low-risk code-wise to me for a significant win.

What's the process for patching RC at this point? Is it just to merge it into master like was done and then cut a patch release for 4.6 that Core will use?

@afercia afercia mentioned this pull request Nov 28, 2018
Closed
@afercia
Copy link
Contributor

afercia commented Nov 28, 2018 •
edited
Loading

I thought anything that gets committed in 4.6 is meant to be merged in WP 5.0?

https://wordpress.slack.com/archives/C02QB2JS7/p1542908760181400

@tofumatt
Copy link
Member

tofumatt commented Nov 28, 2018 •
edited
Loading

I think the idea would be something like a 4.5.2, based on that Slack discussion, would have fixes like this cherry-picked if approved by the release lead (@m). I think all commits against 5.0 now need to go through the appropriate release lead (for a11y, me) and @pento or @m: https://wordpress.slack.com/archives/C02RQBWTW/p1542966281138400

I think this would be worth including as the fix is low-risk for high-reward.

@mtias mtias added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 29, 2018
daniloercoli added a commit that referenced this pull request Nov 29, 2018
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
c0748b5 
…rnmobile/danilo-try-to-fix-undo-redo

* 'master' of https://github.com/WordPress/gutenberg:
  Avoid changing default wpautop priority (#11722)
  Try fixing the tab navigation issue (#12390)
  Polish editor style documentation (#12381)
@mtias mtias modified the milestones: 4.6, 4.7 Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ellatrix ellatrix ellatrix approved these changes

Assignees
No one assigned
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
4.7
Development

Successfully merging this pull request may close these issues.
5 participants
@youknowriad @ellatrix @tofumatt @afercia @mtias