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 keyboard shortcuts #1148

Merged
merged 4 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
36 changes: 34 additions & 2 deletions src/components/ActiveLocales.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,38 @@
setSelectedLanguage( languages.at( -1 ) );
} );

useShortcut( 'preferred-languages/select-previous', ( event ) => {
event.preventDefault();

Check warning on line 65 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L65

Added line #L65 was not covered by tests

if ( isEmpty ) {
return;

Check warning on line 68 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L68

Added line #L68 was not covered by tests
}

const foundIndex = languages.findIndex(
( { locale } ) => locale === selectedLanguage.locale

Check warning on line 72 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L71-L72

Added lines #L71 - L72 were not covered by tests
);

if ( languages[ foundIndex - 1 ] ) {
setSelectedLanguage( languages[ foundIndex - 1 ] );

Check warning on line 76 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L76

Added line #L76 was not covered by tests
}
} );

useShortcut( 'preferred-languages/select-next', ( event ) => {
event.preventDefault();

Check warning on line 81 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L81

Added line #L81 was not covered by tests

if ( isEmpty ) {
return;

Check warning on line 84 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L84

Added line #L84 was not covered by tests
}

const foundIndex = languages.findIndex(
( { locale } ) => locale === selectedLanguage.locale

Check warning on line 88 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L87-L88

Added lines #L87 - L88 were not covered by tests
);

if ( languages[ foundIndex + 1 ] ) {
setSelectedLanguage( languages[ foundIndex + 1 ] );

Check warning on line 92 in src/components/ActiveLocales.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/ActiveLocales.tsx#L92

Added line #L92 was not covered by tests
}
} );

const onRemove = () => {
const foundIndex = languages.findIndex(
( { locale } ) => locale === selectedLanguage.locale
Expand Down Expand Up @@ -248,7 +280,7 @@
<Button
variant="secondary"
showTooltip
aria-keyshortcuts="ArrowUp"
aria-keyshortcuts="Alt+ArrowUp"
aria-label={ sprintf(
/* translators: accessibility text */
__( 'Move up (%s)', 'preferred-languages' ),
Expand All @@ -269,7 +301,7 @@
<Button
variant="secondary"
showTooltip
aria-keyshortcuts="ArrowDown"
aria-keyshortcuts="Alt+ArrowDown"
aria-label={ sprintf(
/* translators: accessibility text */
__( 'Move down (%s)', 'preferred-languages' ),
Expand Down
23 changes: 23 additions & 0 deletions src/components/PreferredLanguages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,34 @@ function PreferredLanguages( props: PreferredLanguagesProps ) {

const { registerShortcut } = useDispatch( keyboardShortcutsStore );
useEffect( () => {
registerShortcut( {
name: 'preferred-languages/select-previous',
category: 'global',
description: __(
'Select previous language',
'preferred-languages'
),
keyCombination: {
character: 'ArrowUp',
},
} );

registerShortcut( {
name: 'preferred-languages/select-next',
category: 'global',
description: __( 'Select next language', 'preferred-languages' ),
keyCombination: {
character: 'ArrowDown',
},
} );

registerShortcut( {
name: 'preferred-languages/move-up',
category: 'global',
description: __( 'Move language up', 'preferred-languages' ),
keyCombination: {
character: 'ArrowUp',
modifier: 'alt',
},
} );

Expand All @@ -66,6 +88,7 @@ function PreferredLanguages( props: PreferredLanguagesProps ) {
description: __( 'Move language down', 'preferred-languages' ),
keyCombination: {
character: 'ArrowDown',
modifier: 'alt',
},
} );

Expand Down
2 changes: 2 additions & 0 deletions src/components/test/PreferredLanguages.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,15 @@ function moveUp() {
fireEvent.keyDown( screen.getByRole( 'listbox' ), {
key: 'ArrowUp',
keyCode: UP,
altKey: true,
} );
}

function moveDown() {
fireEvent.keyDown( screen.getByRole( 'listbox' ), {
key: 'ArrowDown',
keyCode: DOWN,
altKey: true,
} );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Description: For testing purposes only.
Version: 1.0.0
Text Domain: custom-internationalized-plugin
Domain Path: languages/
Domain Path: /languages
*/

function custom_i18n_plugin_test() {
Expand Down
6 changes: 3 additions & 3 deletions tests/e2e/specs/settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ test.describe( 'Settings Page', () => {
).toBeEnabled();

// After moving one position up, none of the buttons should be disabled anymore.
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'Alt+ArrowUp' );

await expect(
page.getByRole( 'button', { name: /Move up/ } )
Expand All @@ -144,8 +144,8 @@ test.describe( 'Settings Page', () => {
).toBeEnabled();

// After moving two up again, the "Move Up" button should be disabled because we reached the top.
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'Alt+ArrowUp' );
await page.keyboard.press( 'Alt+ArrowUp' );

await expect(
page.getByRole( 'button', { name: /Move up/ } )
Expand Down
Loading