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

Fix navigate regions backwards for macOS Firefox and Safari. #45019

Merged
merged 7 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

### Bug Fix

- `useNavigateRegions`: Add new keyboard shortcut alias to cover backtick and tilde keys inconsistencies across browsers ([#45019](https://github.com/WordPress/gutenberg/pull/45019)).
- `Button`: Tweak the destructive button primary, link, and default variants ([#44427](https://github.com/WordPress/gutenberg/pull/44427)).
- `UnitControl`: Fix `disabled` style is overridden by core `form.css` style ([#45250](https://github.com/WordPress/gutenberg/pull/45250)).
- `ItemGroup`: fix RTL `Item` styles when rendered as a button ([#45280](https://github.com/WordPress/gutenberg/pull/45280)).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const defaultShortcuts = {
modifier: 'ctrlShift',
character: '`',
},
{
modifier: 'ctrlShift',
character: '~',
},
{
modifier: 'access',
character: 'p',
Expand Down
4 changes: 4 additions & 0 deletions packages/edit-post/src/components/keyboard-shortcuts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ function KeyboardShortcuts() {
modifier: 'access',
character: 'p',
},
{
modifier: 'ctrlShift',
character: '~',
},
],
} );

Expand Down
4 changes: 4 additions & 0 deletions packages/edit-site/src/components/keyboard-shortcuts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ function KeyboardShortcutsRegister() {
modifier: 'access',
character: 'p',
},
{
modifier: 'ctrlShift',
character: '~',
},
],
} );
registerShortcut( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export default function useRegisterShortcuts() {
modifier: 'access',
character: 'p',
},
{
modifier: 'ctrlShift',
character: '~',
},
],
} );
}, [] );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ function KeyboardShortcutsRegister() {
modifier: 'access',
character: 'p',
},
{
modifier: 'ctrlShift',
character: '~',
},
],
} );
}, [ registerShortcut ] );
Expand Down
51 changes: 51 additions & 0 deletions test/e2e/specs/editor/various/a11y-region-navigation.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* WordPress dependencies
*/
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );

test.describe( 'Region navigation (@firefox, @webkit)', () => {
test.beforeEach( async ( { admin } ) => {
await admin.createNewPost();
} );

test.afterAll( async ( { requestUtils } ) => {
await requestUtils.deleteAllPosts();
} );

test( 'navigates forward and back again', async ( {
editor,
page,
}, testInfo ) => {
// Insert a paragraph block.
await editor.insertBlock( {
name: 'core/paragraph',
attributes: { content: 'Dummy text' },
} );

// Navigate to first region and check that we made it.
await page.keyboard.press( 'Control+`' );
const editorTopBar = page.locator(
'role=region[name="Editor top bar"i]'
);
await expect( editorTopBar ).toBeFocused();

// Navigate to next/second region and check that we made it.
await page.keyboard.press( 'Control+`' );
const editorContent = page.locator(
'role=region[name="Editor content"i]'
);
await expect( editorContent ).toBeFocused();

// Navigate to previous/first region and check that we made it.
// Make sure navigating backwards works also witht he tilde character,
Copy link
Member

Choose a reason for hiding this comment

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

Nit: typo

Suggested change
// Make sure navigating backwards works also witht he tilde character,
// Make sure navigating backwards works also with the tilde character,

// as browsers interpret the combination of the crtl+shift+backtick keys
// and assign it to event.key inconsistently.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Might be worth adding a link to this PR in the comment here. It's optional though as git blame will do the same 😅 .

if ( testInfo.project.name === 'chromium' ) {
await page.keyboard.press( 'Control+Shift+`' );
} else {
await page.keyboard.press( 'Control+Shift+~' );
}

await expect( editorTopBar ).toBeFocused();
} );
} );