Skip to content

Commit

Permalink
Fix few issues with focus management. Fix #519. Fix #339.
Browse files Browse the repository at this point in the history
* Fix unnecesary tab stops
* Fix tabbing back to an item not focusing on the active element in some
  scenarios
  • Loading branch information
tnajdek committed Aug 29, 2024
1 parent 374ecf3 commit 4fddcbc
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 1 deletion.
2 changes: 1 addition & 1 deletion modules/web-common
5 changes: 5 additions & 0 deletions src/js/component/item/items/table.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,11 @@ const Table = () => {
}, [dispatch, isEmbedded, focusBySelector]);

const handleTableFocus = useCallback(ev => {
// The overflow list is made focusable by default in most browsers and to prevent this we
// need to set `tabIndex` to -1. However, since this element is rendered by react-window, we
// cannot pass it as a prop. As a workaround, we set the tabIndex attribute directly here.
// This fixes issue #519.
outerRef.current?.setAttribute?.('tabindex', '-1');
const hasChangedFocused = receiveFocus(ev);
if(hasChangedFocused) {
dispatch(triggerFocus('items-table', true));
Expand Down
1 change: 1 addition & 0 deletions src/js/component/tag-selector/tag-selector-items.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ const TagSelectorItems = () => {

return (
<div
tabIndex="-1"
className="scroll-container"
onScroll={ maybeLoadMore }
ref={ containerRef }
Expand Down
58 changes: 58 additions & 0 deletions test/keyboard-ui-reverse.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* @jest-environment ./test/utils/zotero-css-env.js
*/
import '@testing-library/jest-dom';
import { getByRole, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event'

import { renderWithProviders } from './utils/render';
import { JSONtoState } from './utils/state';
import { MainZotero } from '../src/js/component/main';
import { applyAdditionalJestTweaks, waitForPosition } from './utils/common';
import stateRaw from './fixtures/state/desktop-test-user-item-view.json';

const state = JSONtoState(stateRaw);

// these tests include styles which makes them very slow
applyAdditionalJestTweaks({ timeout: 240000 });

test('Tabulate back through the UI', async () => {
delete window.location;
window.location = new URL('http://localhost/testuser/collections/WTTJ2J56/items/VR82JUX8/item-details');
const user = userEvent.setup()
renderWithProviders(<MainZotero />, { preloadedState: state, includeStyles: true });

await waitForPosition();
await user.click(screen.getByRole('textbox', { name: 'Title' }));
expect(screen.getByRole('textbox', { name: 'Title' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
const inputTypeCombo = screen.getByRole('combobox', { name: 'Item Type' });
expect(getByRole(inputTypeCombo, 'textbox')).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('tab', { name: 'Info' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('row', { name: 'Effects of diet restriction on life span and age-related changes in dogs' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('button', { name: 'New Item' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('searchbox', { name: 'Filter Tags' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('button', { name: 'cute' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('button', { name: 'Collapse Tag Selector' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('treeitem', { name: 'My Library' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('searchbox', { name: 'Title, Creator, Year' })).toHaveFocus();
});


12 changes: 12 additions & 0 deletions test/keyboard-ui.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,16 @@ test('Tabulate through the UI', async () => {
expect(screen.getAllByRole('combobox', { name: 'Creator Type' }).shift()).toHaveFocus();
});

test('Tabbing back to item details tabs should focus on the last selected tab', async () => {
delete window.location;
window.location = new URL('http://localhost/testuser/collections/WTTJ2J56/items/VR82JUX8/item-details');
const user = userEvent.setup()
renderWithProviders(<MainZotero />, { preloadedState: state, includeStyles: true });

await user.click(screen.getByRole('tab', { name: 'Tags' }));
await user.keyboard('{tab}');
expect(screen.getByRole('button', { name: 'Add Tag' })).toHaveFocus();

await user.keyboard('{shift>}{tab}{/shift}');
expect(screen.getByRole('tab', { name: 'Tags' })).toHaveFocus();
});

0 comments on commit 4fddcbc

Please sign in to comment.