-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigator: restore focus only once per location #44972
Conversation
Size Change: +29 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
210a6a6
to
80b8c21
Compare
it( 'should keep focus on the element that is being interacted with, while re-rendering', async () => { | ||
const user = userEvent.setup( { | ||
advanceTimers: jest.advanceTimersByTime, | ||
describe( 'focus management', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this file are:
- moving tests around (into the new
describe
for focus management) - removing unnecessary assertions (those assertions are already made in other tests in the same file)
- adding a new test specifically for the bug that this PR is fixing
Reviewing the PR commit-by-commit should make it easier to review them
…e on the same screen
6ff0adf
to
2c05f41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix @ciampo!
✅ Code changes look good
✅ Unit tests are green
✅ Can replicate original issue
✅ This PR resolves issue
There was one minor nit/typo but other than that this looks good to me. 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍
Co-authored-by: Aaron Robertshaw <[email protected]>
I just cherry-picked this PR to the wp/6.1-rc-2 branch to get it included in the next release: a68fa58 |
Package updates for bug and regression fixes: - `@wordpress/block-directory: 3.15.8` - `@wordpress/block-editor: 10.0.7` - `@wordpress/block-library: 7.14.8` - `@wordpress/components: 21.0.6` - `@wordpress/customize-widgets: 3.14.8` - `@wordpress/edit-post: 6.14.8` - `@wordpress/edit-site: 4.14.10` - `@wordpress/edit-widgets: 4.14.8` - `@wordpress/editor: 12.16.7` - `@wordpress/format-library: 3.15.7` - `@wordpress/interface: 4.16.6` - `@wordpress/list-reusable-blocks: 3.15.6` - `@wordpress/nux: 5.15.6` - `@wordpress/preferences: 2.9.6` - `@wordpress/reusable-blocks: 3.15.7` - `@wordpress/server-side-render: 3.15.6` - `@wordpress/widgets: 2.15.7` Original PRs from Gutenberg repository: - [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined] - [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks] - [WordPress/gutenberg#44999 #44999 Escape comment author URL] - [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location] - [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing] - [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked] - [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block] - [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item] - [WordPress/gutenberg#44853 #44853 Fix overflowing patterns] - [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender] - [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string] Follow-up to [54257], [54335], [54383], [54483], [54486], [54490]. Props bernhard-reiter, audrasjb. See #56467. Built from https://develop.svn.wordpress.org/trunk@54632 git-svn-id: http://core.svn.wordpress.org/trunk@54184 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Package updates for bug and regression fixes: - `@wordpress/block-directory: 3.15.8` - `@wordpress/block-editor: 10.0.7` - `@wordpress/block-library: 7.14.8` - `@wordpress/components: 21.0.6` - `@wordpress/customize-widgets: 3.14.8` - `@wordpress/edit-post: 6.14.8` - `@wordpress/edit-site: 4.14.10` - `@wordpress/edit-widgets: 4.14.8` - `@wordpress/editor: 12.16.7` - `@wordpress/format-library: 3.15.7` - `@wordpress/interface: 4.16.6` - `@wordpress/list-reusable-blocks: 3.15.6` - `@wordpress/nux: 5.15.6` - `@wordpress/preferences: 2.9.6` - `@wordpress/reusable-blocks: 3.15.7` - `@wordpress/server-side-render: 3.15.6` - `@wordpress/widgets: 2.15.7` Original PRs from Gutenberg repository: - [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined] - [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks] - [WordPress/gutenberg#44999 #44999 Escape comment author URL] - [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location] - [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing] - [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked] - [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block] - [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item] - [WordPress/gutenberg#44853 #44853 Fix overflowing patterns] - [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender] - [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string] Follow-up to [54257], [54335], [54383], [54483], [54486], [54490]. Props bernhard-reiter, audrasjb. See #56467. Built from https://develop.svn.wordpress.org/trunk@54632 git-svn-id: https://core.svn.wordpress.org/trunk@54184 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Package updates for bug and regression fixes: - `@wordpress/block-directory: 3.15.8` - `@wordpress/block-editor: 10.0.7` - `@wordpress/block-library: 7.14.8` - `@wordpress/components: 21.0.6` - `@wordpress/customize-widgets: 3.14.8` - `@wordpress/edit-post: 6.14.8` - `@wordpress/edit-site: 4.14.10` - `@wordpress/edit-widgets: 4.14.8` - `@wordpress/editor: 12.16.7` - `@wordpress/format-library: 3.15.7` - `@wordpress/interface: 4.16.6` - `@wordpress/list-reusable-blocks: 3.15.6` - `@wordpress/nux: 5.15.6` - `@wordpress/preferences: 2.9.6` - `@wordpress/reusable-blocks: 3.15.7` - `@wordpress/server-side-render: 3.15.6` - `@wordpress/widgets: 2.15.7` Original PRs from Gutenberg repository: - [WordPress/gutenberg#45041 #45041 Font Size Picker Hint: Fallback to font size `slug` if `name` is undefined] - [WordPress/gutenberg#45045 #45045 Add: Missing output escaping on some blocks] - [WordPress/gutenberg#44999 #44999 Escape comment author URL] - [WordPress/gutenberg#44972 #44972 Navigator: restore focus only once per location] - [WordPress/gutenberg#44858 #44858 Spacing Sizes Control: Try improving layout spacing] - [WordPress/gutenberg#44878 #44878 Fix: Inspector is usable on the top level block even if it is content locked] - [WordPress/gutenberg#44809 #44809 Fix list outdents on Enter in quote block] - [WordPress/gutenberg#44864 #44864 List v2: fix selection when creating paragraph from empty list item] - [WordPress/gutenberg#44853 #44853 Fix overflowing patterns] - [WordPress/gutenberg#45050 #45050 Fix visibility of nested Group block appender] - [WordPress/gutenberg#44887 #44887 wp-env: Use case insensitive regex when checking WP version string] Follow-up to [54257], [54335], [54383], [54483], [54486], [54490]. Props bernhard-reiter, audrasjb. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54632 602fd350-edb4-49c9-b593-d223f7449a82
What?
Requires changes from #44970
Fixes #44834
Fix a bug which caused
Navigator
to attempt to move focus in a screen more than once while the same screen is being re-rendered.This focus shifting can impact the user experience, especially in cases there the user is interacting with an input, and changes to the input cause the screen to re-render.
Why?
These changes fix an unintended behaviour of the component.
We were already trying to solve this issue with the changes from #44239, but #44834 highlights an edge case that wasn't covered yet: the input that the user is interacting with is not rendered inside of
Navigator
.How?
A new
hasRestoredFocus
flag has been added tolocation
objects, and is used to avoid that a screen tries to move focus more than once for the same location.Testing Instructions
Follow instructions from #44834 , the bug should be fixed
I've also added a new unit test to cover this edge case.
Screenshots or screencast
Before:
Kapture.2022-10-10.at.18.28.42.mp4
After:
Kapture.2022-10-14.at.12.56.21.mp4