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: disable browser autocomplete and edit dropdown items elements #2513

Merged
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
17 changes: 9 additions & 8 deletions src/Form/FormAutosuggest.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function FormAutosuggest({
});

const handleItemClick = (e, onClick) => {
const clickedValue = e.currentTarget.value;
const clickedValue = e.currentTarget.getAttribute('data-value');

if (onSelected && clickedValue !== value) {
onSelected(clickedValue);
Expand All @@ -66,7 +66,7 @@ function FormAutosuggest({
return React.cloneElement(child, {
...rest,
children,
value: children,
'data-value': children,
onClick: (e) => handleItemClick(e, onClick),
});
});
Expand Down Expand Up @@ -219,6 +219,9 @@ function FormAutosuggest({
<FormControl
aria-expanded={(state.dropDownItems.length > 0).toString()}
aria-owns="pgn__form-autosuggest__dropdown-box"
role="combobox"
aria-autocomplete="list"
autoComplete="off"
value={state.displayValue}
aria-invalid={state.errorMessage}
onChange={handleOnChange}
Expand All @@ -240,25 +243,25 @@ function FormAutosuggest({
)}
</FormGroup>

<div
<ul
id="pgn__form-autosuggest__dropdown-box"
className="pgn__form-autosuggest__dropdown"
role="listbox"
>
{isLoading ? (
<div className="pgn__form-autosuggest__dropdown-loading">
<Spinner animation="border" variant="dark" screenReaderText={screenReaderText} />
</div>
) : state.dropDownItems.length > 0 && state.dropDownItems}
</div>
</ul>
</div>
);
}

FormAutosuggest.defaultProps = {
arrowKeyNavigationSelector: 'a:not(:disabled),button:not(:disabled, .btn-icon),input:not(:disabled)',
arrowKeyNavigationSelector: 'a:not(:disabled),li:not(:disabled, .btn-icon),input:not(:disabled)',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We realized that after disabling the browser autocomplete that changing button to li broke the keyboard navigation through the dropdown items but this was easily solved by changing this line to include li.

ignoredArrowKeysNames: ['ArrowRight', 'ArrowLeft'],
isLoading: false,
role: 'list',
className: null,
floatingLabel: null,
onChange: null,
Expand All @@ -283,8 +286,6 @@ FormAutosuggest.propTypes = {
ignoredArrowKeysNames: PropTypes.arrayOf(PropTypes.string),
/** Specifies loading state. */
isLoading: PropTypes.bool,
/** An ARIA role describing the form autosuggest. */
role: PropTypes.string,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed this during the Paragon Working Group. We made the decision to remove consumers' ability to change the role to something that would break WCAG.

/** Specifies class name to append to the base element. */
className: PropTypes.string,
/** Specifies floating label to display for the input component. */
Expand Down
3 changes: 3 additions & 0 deletions src/Form/FormAutosuggestOption.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ function FormAutosuggestOption({
}) {
return (
<MenuItem
as="li"
role="option"
tabindex="-1"
onClick={onClick}
className={classNames(className, 'dropdown-item')}
{...props}
Expand Down
1 change: 1 addition & 0 deletions src/Form/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ select.form-control {
width: calc(100% - .5rem);
z-index: $zindex-dropdown;
top: 3.125rem;
padding: 0;

.dropdown-item {
display: block;
Expand Down
2 changes: 1 addition & 1 deletion src/Form/form-autosuggest.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Form auto-suggest enables users to manually select or type to find matching opti
<Form.AutosuggestOption>JavaScript</Form.AutosuggestOption>
<Form.AutosuggestOption>Python</Form.AutosuggestOption>
<Form.AutosuggestOption>Rube</Form.AutosuggestOption>
<Form.AutosuggestOption onClick={(e) => alert(e.currentTarget.value)}>
<Form.AutosuggestOption onClick={(e) => alert(e.currentTarget.getAttribute('data-value'))}>
Option with custom onClick
</Form.AutosuggestOption>
</Form.Autosuggest>
Expand Down
22 changes: 11 additions & 11 deletions src/Form/tests/FormAutosuggest.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('FormAutosuggest', () => {

it('renders component with options', () => {
container.find('input').simulate('click');
const optionsList = container.find('.pgn__form-autosuggest__dropdown').find('button');
const optionsList = container.find('.pgn__form-autosuggest__dropdown').find('li');

expect(optionsList.length).toEqual(3);
});
Expand All @@ -94,7 +94,7 @@ describe('FormAutosuggest', () => {
describe('controlled behavior', () => {
it('selects option', () => {
container.find('input').simulate('click');
container.find('.pgn__form-autosuggest__dropdown').find('button')
container.find('.pgn__form-autosuggest__dropdown').find('li')
.at(0).simulate('click');

expect(container.find('input').instance().value).toEqual('Option 1');
Expand All @@ -104,15 +104,15 @@ describe('FormAutosuggest', () => {

it('when a function is passed to onClick, it is called', () => {
container.find('input').simulate('change', { target: { value: 'Option 2' } });
container.find('.pgn__form-autosuggest__dropdown').find('button')
container.find('.pgn__form-autosuggest__dropdown').find('li')
.at(0).simulate('click');

expect(onClick).toHaveBeenCalledTimes(1);
});

it('when a function is not passed to onClick, it is not called', () => {
container.find('input').simulate('change', { target: { value: 'Option 1' } });
container.find('.pgn__form-autosuggest__dropdown').find('button')
container.find('.pgn__form-autosuggest__dropdown').find('li')
.at(0).simulate('click');

expect(onClick).toHaveBeenCalledTimes(0);
Expand All @@ -127,39 +127,39 @@ describe('FormAutosuggest', () => {
it('options list depends on filled field value', () => {
container.find('input').simulate('change', { target: { value: 'option 1' } });

expect(container.find('.pgn__form-autosuggest__dropdown').find('button').length).toEqual(1);
expect(container.find('.pgn__form-autosuggest__dropdown').find('li').length).toEqual(1);
expect(onSelected).toHaveBeenCalledTimes(0);
});

it('toggles options list', () => {
const dropdownContainer = '.pgn__form-autosuggest__dropdown';

expect(container.find(dropdownContainer).find('button').length).toEqual(1);
expect(container.find(dropdownContainer).find('li').length).toEqual(1);

container.find('button.pgn__form-autosuggest__icon-button').simulate('click');
expect(container.find(dropdownContainer).find('button').length).toEqual(0);
expect(container.find(dropdownContainer).find('li').length).toEqual(0);

container.find('button.pgn__form-autosuggest__icon-button').simulate('click');
expect(container.find(dropdownContainer).find('button').length).toEqual(1);
expect(container.find(dropdownContainer).find('li').length).toEqual(1);
});

it('shows options list depends on field value', () => {
container.find('input').simulate('change', { target: { value: '1' } });

expect(container.find('.pgn__form-autosuggest__dropdown').find('button').length).toEqual(2);
expect(container.find('.pgn__form-autosuggest__dropdown').find('li').length).toEqual(2);
});

it('closes options list on click outside', () => {
const fireEvent = createDocumentListenersMock();
const dropdownContainer = '.pgn__form-autosuggest__dropdown';

container.find('input').simulate('click');
expect(container.find(dropdownContainer).find('button').length).toEqual(2);
expect(container.find(dropdownContainer).find('li').length).toEqual(2);

act(() => { fireEvent.click(document.body); });
container.update();

expect(container.find(dropdownContainer).find('button').length).toEqual(0);
expect(container.find(dropdownContainer).find('li').length).toEqual(0);
});
});
});
Loading