maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Improve accessibility of top search bar (#14263)

Browse source 
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Nick Taylor <nick@iamdeveloper.com>
This commit is contained in:
Jeferson S. Brito 2021-08-09 10:55:41 -03:00 • committed by GitHub
parent 2ccac74ad6
commit b61dc606c3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 126 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/assets/stylesheets/components/header.scss
View file

@ -44,6 +44,10 @@
margin: 0 var(--su-4);
display: block;
}
.crayons-btn:focus {
outline: revert;
}
}
.crayons-header__link {

26
app/javascript/Search/Search.jsx
View file

@ -11,7 +11,6 @@ import { SearchForm } from './SearchForm';
const GLOBAL_MINIMIZE_KEY = 'Digit0';
const GLOBAL_SEARCH_KEY = '/';
const ENTER_KEY = 'Enter';
export class Search extends Component {
constructor(props) {
@ -74,27 +73,21 @@ export class Search extends Component {
};
submit = (event) => {
if (hasInstantClick) {
event.preventDefault();
}
};
event.preventDefault();
search(key, searchTerm) {
const { value: searchTerm } = this.searchInputRef.current;
const { searchTerm: currentSearchTerm } = this.props;
this.enableSearchPageChecker = false;
if (
hasInstantClick() &&
key === ENTER_KEY &&
currentSearchTerm !== searchTerm
) {
if (hasInstantClick() && searchTerm !== currentSearchTerm) {
const { setSearchTerm } = this.props;
setSearchTerm(searchTerm);
preloadSearchResults({ searchTerm });
displaySearchResults({ searchTerm });
}
}
};
componentWillUnmount() {
document.removeEventListener('keydown', this.globalKeysListener);
@ -128,13 +121,6 @@ export class Search extends Component {
/>
<SearchForm
searchTerm={searchTerm}
onSearch={(event) => {
const {
key,
target: { value },
} = event;
this.search(key, value);
}}
onSubmitSearch={this.submit}
ref={this.searchInputRef}
/>

69
app/javascript/Search/SearchForm.jsx
View file

@ -1,33 +1,56 @@
import PropTypes from 'prop-types';
import { h } from 'preact';
import { forwardRef } from 'preact/compat';
import { Button } from '@crayons';
export const SearchForm = forwardRef(
({ searchTerm, onSearch, onSubmitSearch }, ref) => (
<form
action="/search"
acceptCharset="UTF-8"
method="get"
onSubmit={onSubmitSearch}
>
<input name="utf8" type="hidden" value="✓" />
<input
ref={ref}
className="crayons-header--search-input crayons-textfield"
type="text"
name="q"
placeholder="Search..."
autoComplete="off"
aria-label="search"
onKeyDown={onSearch}
value={searchTerm}
/>
</form>
),
const SearchIcon = () => (
<svg
width="24"
height="24"
viewBox="0 0 24 24"
className="crayons-icon"
xmlns="http://www.w3.org/2000/svg"
>
<path d="M18.031 16.617l4.283 4.282-1.415 1.415-4.282-4.283A8.96 8.96 0 0111 20c-4.968 0-9-4.032-9-9s4.032-9 9-9 9 4.032 9 9a8.96 8.96 0 01-1.969 5.617zm-2.006-.742A6.977 6.977 0 0018 11c0-3.868-3.133-7-7-7-3.868 0-7 3.132-7 7 0 3.867 3.132 7 7 7a6.977 6.977 0 004.875-1.975l.15-.15z" />
</svg>
);
export const SearchForm = forwardRef(({ searchTerm, onSubmitSearch }, ref) => (
<form
action="/search"
acceptCharset="UTF-8"
method="get"
onSubmit={onSubmitSearch}
role="search"
>
<input name="utf8" type="hidden" value="✓" />
<div class="crayons-fields crayons-fields--horizontal">
<div class="crayons-field flex-1 relative">
<input
ref={ref}
className="crayons-header--search-input crayons-textfield"
type="text"
name="q"
placeholder="Search..."
autoComplete="off"
aria-label="Search term"
value={searchTerm}
/>
<Button
type="submit"
variant="ghost"
contentType="icon-rounded"
icon={SearchIcon}
size="s"
className="absolute right-2 bottom-0 top-0 m-1"
aria-label="Search"
/>
</div>
</div>
</form>
));
SearchForm.propTypes = {
searchTerm: PropTypes.string.isRequired,
onSearch: PropTypes.func.isRequired,
onSubmitSearch: PropTypes.func.isRequired,
};

3
app/javascript/Search/__stories__/SearchForm.stories.jsx
View file

@ -4,7 +4,6 @@ import { action } from '@storybook/addon-actions';
import { SearchForm } from '..';
const commonProps = {
onSearch: action('on preloading search'),
onSubmitSearch: (e) => {
e.preventDefault();
action('on submit')(e);
@ -13,7 +12,7 @@ const commonProps = {
class FocusedForm extends Component {
componentDidMount() {
document.getElementById('nav-search').focus();
document.querySelector('.crayons-header--search-input').focus();
}
render() {

72
app/javascript/Search/__tests__/Search.test.jsx
View file

@ -1,5 +1,6 @@
import { h } from 'preact';
import { render, fireEvent } from '@testing-library/preact';
import { render, fireEvent, waitFor } from '@testing-library/preact';
import userEvent from '@testing-library/user-event';
import { axe } from 'jest-axe';
import { Search } from '../Search';
@ -34,9 +35,9 @@ describe('<Search />', () => {
setSearchTerm: jest.fn(),
};
const { getByLabelText } = render(<Search {...props} />);
const { getByRole } = render(<Search {...props} />);
const searchInput = getByLabelText(/search/i);
const searchInput = getByRole('textbox', { name: /search/i });
expect(searchInput.value).toEqual('fish');
expect(searchInput.getAttribute('placeholder')).toEqual('Search...');
@ -48,23 +49,15 @@ describe('<Search />', () => {
searchTerm: 'fish',
setSearchTerm: jest.fn(),
};
const { getByLabelText, findByLabelText } = render(<Search {...props} />);
const { getByRole, findByRole } = render(<Search {...props} />);
let searchInput = getByLabelText(/search/i);
let searchInput = getByRole('textbox', { name: /search/i });
expect(searchInput.value).toEqual('fish');
// user.type doesn't work in the case of
// search as the current implementation is relying on keydown
// events
fireEvent.keyDown(searchInput, {
key: 'Enter',
keyCode: 13,
which: 13,
target: { value: 'hello' },
});
fireEvent.change(searchInput, { target: { value: 'hello' } });
searchInput = await findByLabelText(/search/i);
searchInput = await findByRole('textbox', { name: /search/i });
expect(searchInput.value).toEqual('hello');
});
@ -74,53 +67,42 @@ describe('<Search />', () => {
searchTerm: '',
setSearchTerm: jest.fn(),
};
const { getByLabelText } = render(<Search {...props} />);
const { getByRole } = render(<Search {...props} />);
let searchInput = getByLabelText(/search/i);
const searchInput = getByRole('textbox', { name: /search/i });
expect(searchInput.value).toEqual('');
// user.type doesn't work in the case of
// search as the current implementation is relying on keydown
// events
fireEvent.keyDown(searchInput, {
key: 'Enter',
keyCode: 13,
which: 13,
target: { value: 'hello' },
});
userEvent.type(searchInput, 'hello');
expect(searchInput.value).toEqual('hello');
expect(props.setSearchTerm).toHaveBeenCalledWith('hello');
waitFor(() => {
expect(searchInput.value).toEqual('hello');
expect(props.setSearchTerm).toHaveBeenCalledWith('hello');
});
});
it('should submit the search form', async () => {
jest.spyOn(Search.prototype, 'search');
const props = {
searchTerm: '',
setSearchTerm: jest.fn(),
onSubmitSearch: jest.fn(),
};
const { getByLabelText, findByLabelText } = render(<Search {...props} />);
const { getByRole, findByRole } = render(<Search {...props} />);
let searchInput = getByLabelText(/search/i);
let searchInput = getByRole('textbox', { name: /search/i });
expect(searchInput.value).toEqual('');
// user.type doesn't work in the case of
// search as the current implementation is relying on keydown
// events
fireEvent.keyDown(searchInput, {
key: 'Enter',
keyCode: 13,
which: 13,
target: { value: 'hello' },
userEvent.type(searchInput, 'hello');
fireEvent.submit(getByRole('search'));
searchInput = await findByRole('textbox', { name: /search/i });
waitFor(() => {
expect(searchInput.value).toEqual('hello');
expect(props.onSubmitSearch).toHaveBeenCalledWith('hello');
});
searchInput = await findByLabelText(/search/i);
expect(searchInput.value).toEqual('hello');
expect(Search.prototype.search).toHaveBeenCalledWith('Enter', 'hello');
});
it('should be listening for history state changes', async () => {

20
app/javascript/Search/__tests__/SearchFormSync.test.jsx
View file

@ -28,11 +28,11 @@ describe('<SearchFormSync />', () => {
});
it('should synchronize search forms', async () => {
const { findByLabelText, findAllByLabelText } = render(<SearchFormSync />);
const { getByRole, getAllByRole } = render(<SearchFormSync />);
// Only one input is rendered at this point because the synchSearchForms custom event is what
// tells us that there is a new search form to sync with the existing one.
const searchInput = await findByLabelText('search');
const searchInput = await getByRole('textbox', { name: /search/i });
// Because window.location has no search term in it's URL
expect(searchInput.value).toEqual('');
@ -50,18 +50,20 @@ describe('<SearchFormSync />', () => {
}),
);
const [desktopSearch, mobileSearch] = await findAllByLabelText('search');
const [desktopSearch, mobileSearch] = await getAllByRole('textbox', {
name: /search/i,
});
expect(desktopSearch.value).toEqual(searchTerm);
expect(mobileSearch.value).toEqual(searchTerm);
});
it('should synchronize search forms on a subsequent search', async () => {
const { findByLabelText, findAllByLabelText } = render(<SearchFormSync />);
const { getByRole, getAllByRole } = render(<SearchFormSync />);
// Only one input is rendered at this point because the synchSearchForms custom event is what
// tells us that there is a new search form to sync with the existing one.
const searchInput = await findByLabelText('search');
const searchInput = await getByRole('textbox', { name: /search/i });
// Because window.location has no search term in it's URL
expect(searchInput.value).toEqual('');
@ -79,7 +81,9 @@ describe('<SearchFormSync />', () => {
}),
);
let [desktopSearch, mobileSearch] = await findAllByLabelText('search');
let [desktopSearch, mobileSearch] = await getAllByRole('textbox', {
name: /search/i,
});
expect(desktopSearch.value).toEqual(searchTerm);
expect(mobileSearch.value).toEqual(searchTerm);
@ -107,7 +111,9 @@ describe('<SearchFormSync />', () => {
}),
);
[desktopSearch, mobileSearch] = await findAllByLabelText('search');
[desktopSearch, mobileSearch] = await getAllByRole('textbox', {
name: /search/i,
});
expect(desktopSearch.value).toEqual(searchTerm2);
expect(mobileSearch.value).toEqual(searchTerm2);

11
app/views/layouts/_top_bar.html.erb
View file

@ -20,8 +20,15 @@
</div>
<div class="crayons-header--search js-search-form" id="header-search">
<form accept-charset="UTF-8" method="get" action="<%= search_path %>">
<input class="crayons-header--search-input crayons-textfield js-search-input" type="text" id="nav-search" name="q" placeholder="Search..." autocomplete="off" />
<form accept-charset="UTF-8" method="get" action="<%= search_path %>" role="search">
<div class="crayons-fields crayons-fields--horizontal">
<div class="crayons-field flex-1 relative">
<input class="crayons-header--search-input crayons-textfield js-search-input" type="text" id="nav-search" name="q" placeholder="Search..." autocomplete="off" />
<button type="submit" aria-label="Search" class="crayons-btn crayons-btn--ghost crayons-btn--s crayons-btn--icon-rounded absolute right-2 bottom-0 top-0 m-1">
<%= inline_svg_tag("search.svg", aria_hidden: true, width: 24, height: 24, class: "crayons-icon", title: "Search") %>
</button>
</div>
</div>
</form>
</div>

11
app/views/stories/articles_search/index.html.erb
View file

@ -6,8 +6,15 @@
<main id="main-content">
<div class="crayons-layout crayons-layout--limited-l crayons-layout--1-col p-4">
<div class="block m:hidden" id="mobile-search-container">
<form accept-charset="UTF-8" method="get" action="/search">
<input class="crayons-textfield" type="text" name="q" placeholder="Search..." autocomplete="on" />
<form accept-charset="UTF-8" method="get" action="/search" role="search">
<div class="crayons-fields crayons-fields--horizontal">
<div class="crayons-field flex-1 relative">
<input class="crayons-textfield" type="text" name="q" placeholder="Search..." autocomplete="on" />
<button type="submit" aria-label="Search" class="crayons-btn crayons-btn--ghost crayons-btn--s crayons-btn--icon-rounded absolute right-2 bottom-0 top-0 m-1">
<%= inline_svg_tag("search.svg", aria_hidden: true, width: 24, height: 24, class: "crayons-icon", title: "Search") %>
</button>
</div>
</div>
</form>
</div>

11
cypress/integration/seededFlows/searchFlows/searchForm.spec.js Normal file
View file

@ -0,0 +1,11 @@
describe('Search form', () => {
it('should show results when search button is pressed', () => {
cy.visit('/');
cy.findByRole('textbox', { name: /search/i }).type('test');
cy.findByRole('button', { name: /search/i }).click();
cy.url().should('include', '/search?q=test');
cy.findByRole('heading', { name: 'Test article' }).should('exist');
});
});