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

Keyboard navigation in mobile web (Issue #2159) #2166

Merged
merged 10 commits into from
Oct 13, 2022
Merged

Keyboard navigation in mobile web (Issue #2159) #2166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a58318e
feat: update testing-library/jest-dom + testing-library/react + testi…
LMulvey Oct 11, 2022
5467b7b
feat(Header): improve keyboard accessibility with Header
LMulvey Oct 11, 2022
213746c
feat(Header): Trap focus on mobile navbar while it is open
LMulvey Oct 11, 2022
52bb84b
test: update tests to use new @testing-library/user-event API
LMulvey Oct 11, 2022
4c11f3e
chore: properly lint code and update to please the linter
LMulvey Oct 11, 2022
f8b591b
fix: change desktop displayLinks to use short-circuit evaluation
LMulvey Oct 12, 2022
b148a74
feat: lift up useFocusTrap hook for Modal and Header + fix Header foc…
LMulvey Oct 12, 2022
3be61b1
fix(useFocusTrap): update filtering strategy to only filter display: …
LMulvey Oct 12, 2022
699af39
test(InputTextarea*): remove use of Promise.all and replace with forE…
LMulvey Oct 13, 2022
15dc31d
test(InputSwitch): update test and resolve Todo regarding use of RTL …
LMulvey Oct 13, 2022
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
4 changes: 2 additions & 2 deletions client/app/components/Chart/__tests__/ChartControl.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('ChartControl', () => {
expect(container.querySelector('canvas')).toBeInTheDocument();
});

it('passes down the expected data for the selected type', () => {
it('passes down the expected data for the selected type', async () => {
const data = {
Moments: {
'2013-02-10 00:00:00 -0800': 11,
Expand Down Expand Up @@ -79,7 +79,7 @@ describe('ChartControl', () => {

// simulates a button click for the specified type
const button = screen.getByRole('button', { name: 'Categories' });
userEvent.click(button);
await userEvent.click(button);

// checks that the correct data for the updated type is passed down
expect(AreaChart).toHaveBeenCalledWith(
Expand Down
74 changes: 54 additions & 20 deletions client/app/components/Form/__tests__/DynamicForm.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,13 @@ describe('DynamicForm', () => {
it('has no errors when submit is clicked', async () => {
const axiosPostSpy = jest.spyOn(axios, 'post').mockResolvedValue({});
render(getComponent());
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy).toBeCalled());
});

Expand All @@ -82,7 +87,9 @@ describe('DynamicForm', () => {
.spyOn(axios, 'post')
.mockRejectedValue({ error });
render(getComponent());
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy()).rejects.toEqual({ error }));
expect(getByRole('alert')).toBeInTheDocument();
});
Expand All @@ -92,9 +99,14 @@ describe('DynamicForm', () => {
it('has no errors when submit is clicked', async () => {
const axiosPostSpy = jest.spyOn(axios, 'post').mockResolvedValue({});
render(getComponent());
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.type(getByLabelText('Some Number Label'), '2');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.type(getByLabelText('Some Number Label'), '2');
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy).toBeCalled());
});

Expand All @@ -105,9 +117,14 @@ describe('DynamicForm', () => {
.spyOn(axios, 'post')
.mockRejectedValue({ error });
render(getComponent());
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.type(getByLabelText('Some Number Label'), '-1');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.type(getByLabelText('Some Number Label'), '-1');
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy()).rejects.toEqual({ error }));
expect(getByRole('alert')).toBeInTheDocument();
});
Expand All @@ -128,9 +145,14 @@ describe('DynamicForm', () => {
it('has no errors when submit is clicked', async () => {
const axiosPostSpy = jest.spyOn(axios, 'post').mockResolvedValue({});
render(getComponent({ nameValue: 'name' }));
userEvent.type(getByText('Name'), 'hi');
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(getByText('Name'), 'hi');
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy).toBeCalled());
});

Expand All @@ -141,7 +163,9 @@ describe('DynamicForm', () => {
.spyOn(axios, 'post')
.mockRejectedValue({ error });
render(getComponent({ nameValue: 'name' }));
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy()).rejects.toEqual({ error }));
expect(getByText('This field cannot be empty!')).toBeInTheDocument();
});
Expand All @@ -151,10 +175,15 @@ describe('DynamicForm', () => {
it('has no errors when submit is clicked', async () => {
const axiosPostSpy = jest.spyOn(axios, 'post').mockResolvedValue({});
render(getComponent({ nameValue: 'name' }));
userEvent.type(getByText('Name'), 'hi');
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.type(getByLabelText('Some Number Label'), '2');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(getByText('Name'), 'hi');
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.type(getByLabelText('Some Number Label'), '2');
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy).toBeCalled());
});

Expand All @@ -165,9 +194,14 @@ describe('DynamicForm', () => {
.spyOn(axios, 'post')
.mockRejectedValue({ error });
render(getComponent({ nameValue: 'name' }));
userEvent.type(getByPlaceholderText('Some Text Placeholder'), 'bye');
userEvent.type(getByLabelText('Some Number Label'), '-1');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'bye',
);
await userEvent.type(getByLabelText('Some Number Label'), '-1');
await userEvent.click(
getByRole('button', { name: 'Some Submit Value' }),
);
await waitFor(() => expect(axiosPostSpy()).rejects.toEqual({ error }));
expect(
getByText(
Expand Down
26 changes: 13 additions & 13 deletions client/app/components/Form/__tests__/Form.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,47 +40,47 @@ describe('Form', () => {
});

describe('for changes on the input with text type', () => {
it('has no errors when submit is clicked', () => {
it('has no errors when submit is clicked', async () => {
render(getComponent());
userEvent.type(
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'randomName',
);
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
expect(queryByRole('alert')).not.toBeInTheDocument();
});

it('has errors when submit is clicked', () => {
it('has errors when submit is clicked', async () => {
const scrollIntoViewMock = jest.fn();
window.HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;
render(getComponent());
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
expect(getByText('This field cannot be empty!')).toBeInTheDocument();
});
});

describe('for changes on the input with number type', () => {
it('has no errors when submit is clicked', () => {
it('has no errors when submit is clicked', async () => {
render(getComponent());
userEvent.type(
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'randomName',
);
userEvent.type(getByLabelText('Some Number Label'), '2');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(getByLabelText('Some Number Label'), '2');
await userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
expect(queryByRole('alert')).not.toBeInTheDocument();
});

it('has errors when submit is clicked', () => {
it('has errors when submit is clicked', async () => {
const scrollIntoViewMock = jest.fn();
window.HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;
render(getComponent());
userEvent.type(
await userEvent.type(
getByPlaceholderText('Some Text Placeholder'),
'randomName',
);
userEvent.type(getByLabelText('Some Number Label'), '-1');
userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
await userEvent.type(getByLabelText('Some Number Label'), '-1');
await userEvent.click(getByRole('button', { name: 'Some Submit Value' }));
expect(
getByText(
'This field must be equal or greater than 0 and equal or less than 2!',
Expand Down
7 changes: 6 additions & 1 deletion client/app/components/Header/HeaderProfile.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ export type Props = {
};

const notificationsElement = (notifications) => (
<button type="button" className="buttonGhostXS" aria-label={notifications} tabIndex={-1}>
<button
type="button"
className="buttonGhostXS"
aria-label={notifications}
tabIndex={-1}
>
<FontAwesomeIcon icon={faBell} />
</button>
);
Expand Down
61 changes: 56 additions & 5 deletions client/app/components/Header/__tests__/Header.spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import Header from 'components/Header';

function setup(jsx) {
return {
user: userEvent.setup(),
...render(jsx),
};
}

const component = (
<Header
home={{ name: 'Home', url: '/some-path' }}
Expand All @@ -16,7 +23,7 @@ const component = (

describe('Header', () => {
it('renders correctly', () => {
render(component);
setup(component);

expect(screen.getByRole('banner')).toBeInTheDocument();
expect(screen.getByRole('navigation')).toBeInTheDocument();
Expand All @@ -28,15 +35,59 @@ describe('Header', () => {
expect(screen.getByRole('link', { name: /link 2/i })).toBeInTheDocument();
});

it('toggles hamburger correctly', () => {
const { container } = render(component);
it('toggles hamburger correctly', async () => {
const { container, user } = setup(component);
const hamburger = container.querySelector('#headerHamburger');
userEvent.click(hamburger);
await user.click(hamburger);

const mobile = container.querySelector('#headerMobile');
expect(mobile).toBeInTheDocument();

userEvent.click(hamburger);
await user.click(hamburger);
expect(mobile).not.toBeInTheDocument();
});

it('toggles hamburger from keyboard input when expected', async () => {
const { container, user } = setup(component);
const hamburger = container.querySelector('#headerHamburger');

await user.keyboard('{Tab}{Tab}');
expect(hamburger).toHaveFocus();
await user.keyboard('{Enter}');
expect(container.querySelector('#headerMobile')).toBeInTheDocument();
await user.keyboard('{Enter}');
expect(container.querySelector('#headerMobile')).not.toBeInTheDocument();
await user.keyboard('[Space]');
expect(container.querySelector('#headerMobile')).toBeInTheDocument();
await user.keyboard('[Space]');
expect(container.querySelector('#headerMobile')).not.toBeInTheDocument();
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain what exactly we're testing when we hit the Space key twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ensuring that it opens AND closes the navbar with the space key!


await user.keyboard('{Tab}');
expect(container.querySelector('#headerMobile')).not.toBeInTheDocument();
});

it('traps focus when the mobile navbar is open', async () => {
const { container, user } = setup(component);
const hamburger = container.querySelector('#headerHamburger');

await user.keyboard('{Tab}{Tab}');
expect(hamburger).toHaveFocus();
await user.keyboard('{Enter}');
expect(container.querySelector('#headerMobile')).toBeInTheDocument();

await user.keyboard('{Tab}');
expect(screen.getByRole('link', { name: /link 1/i })).toHaveFocus();
await user.keyboard('{Tab}');
expect(screen.getByRole('link', { name: /link 2/i })).toHaveFocus();
await user.keyboard('{Tab}');
expect(screen.getByRole('link', { name: /home/i })).toHaveFocus();
await user.keyboard('{Tab}');
expect(hamburger).toHaveFocus();
await user.keyboard('{Tab}');
expect(screen.getByRole('link', { name: /link 1/i })).toHaveFocus();

// Shift-tab back to the hamburger and close the mobile menu
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

await user.keyboard('{Shift>}{Tab}{/Shift}{Enter}');
expect(container.querySelector('#headerMobile')).not.toBeInTheDocument();
});
});
Loading