From ebda3999966124606e89d93e245df5e36fd5585c Mon Sep 17 00:00:00 2001 From: Alex Page Date: Wed, 29 May 2019 14:34:52 -0400 Subject: [PATCH] Fixing accessibility issues in shitlist (#1565) * Fixing accessibility issues in shitlist --- UNRELEASED.md | 1 + a11y_shitlist.json | 496 ------------------ src/components/DropZone/DropZone.tsx | 58 +- .../DropZone/tests/DropZone.test.tsx | 108 ++-- src/components/Form/Form.tsx | 12 +- src/components/Layout/README.md | 32 +- src/components/Modal/README.md | 12 +- .../components/CloseButton/CloseButton.tsx | 13 +- .../Navigation/components/Section/Section.tsx | 1 - src/components/OptionList/OptionList.tsx | 2 +- .../Page/components/Header/Header.tsx | 9 +- src/components/Sheet/README.md | 2 + src/components/Tabs/Tabs.tsx | 16 +- src/components/TextField/README.md | 2 +- src/components/TextField/TextField.tsx | 4 + .../components/SearchField/SearchField.tsx | 11 + src/locales/de.json | 17 +- src/locales/en.json | 18 +- src/locales/es.json | 17 +- src/locales/fr.json | 17 +- src/locales/hi.json | 17 +- src/locales/it.json | 17 +- src/locales/ja.json | 17 +- src/locales/ms.json | 17 +- src/locales/nl.json | 17 +- src/locales/pt-BR.json | 17 +- src/locales/zh-CN.json | 17 +- src/locales/zh-TW.json | 17 +- 28 files changed, 324 insertions(+), 660 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 55342260430..564589ad8b0 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -23,6 +23,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f ### Bug fixes - Removed unnecessary border-radius from `Modal` body ([#1584](https://github.com/Shopify/polaris-react/pull/1584)) +- Fixed accessibility issues in `DropZone`, `Form`, `Modal`, `Section`, `Page`, `Tabs`, `TextField` and `TopBar` ([#1565](https://github.com/Shopify/polaris-react/pull/1565),[#1582](https://github.com/Shopify/polaris-react/pull/1582)). - Fixed inconsistent width depending on your browser/version in `Sheet` ([#1569](https://github.com/Shopify/polaris-react/pull/1569)) - Fixed text and other elements from being selected in Safari when dragging the color picker ([#1562](https://github.com/Shopify/polaris-react/pull/1562)) - Fixed `Banner` `title` overflowing when set to a single long word ([#1553](https://github.com/Shopify/polaris-react/pull/1553)) diff --git a/a11y_shitlist.json b/a11y_shitlist.json index 18f9dcc2d42..06660355f76 100644 --- a/a11y_shitlist.json +++ b/a11y_shitlist.json @@ -1,24 +1,4 @@ { - "all-components-app-provider--with-linkcomponent": [ - { - "code": "WCAG2AA.Principle4.Guideline4_1.4_1_2.H91.Button.Name", - "context": "", - "message": "This button element does not have a name available to an accessibility API. Valid names are: title undefined, element content, aria-label undefined, aria-labelledby undefined.", - "type": "error", - "typeCode": 1, - "selector": "#root > div > div > div > div > div:nth-child(2) > button" - }, { "code": "WCAG2AA.Principle1.Guideline1_3.1_3_1.F92,ARIA4", "context": "
  • ", - "message": "This button element does not have a name available to an accessibility API. Valid names are: title undefined, element content, aria-label undefined, aria-labelledby undefined.", - "type": "error", - "typeCode": 1, - "selector": "#root > div > div > div > div > div:nth-child(2) > button" - }, { "code": "WCAG2AA.Principle1.Guideline1_3.1_3_1.F92,ARIA4", "context": "
  • ) : null; - const dropZoneMarkup = ( -
    - {dragOverlay} - {dragErrorOverlay} -
    {children}
    - - - -
    - ); - - const labelledDropzoneMarkup = label ? ( - - {dropZoneMarkup} - - ) : ( - dropZoneMarkup - ); + const labelValue = label + ? label + : intl.translate('Polaris.DropZone.FileUpload.label'); + const labelHiddenValue = label ? labelHidden : true; return ( - {labelledDropzoneMarkup} + + +
    + {dragOverlay} + {dragErrorOverlay} +
    {children}
    + + + +
    +     +     ); } diff --git a/src/components/DropZone/tests/DropZone.test.tsx b/src/components/DropZone/tests/DropZone.test.tsx index 9c42a85e2cd..0f9a1fe0810 100755 --- a/src/components/DropZone/tests/DropZone.test.tsx +++ b/src/components/DropZone/tests/DropZone.test.tsx @@ -53,19 +53,16 @@ describe('', () => { it('calls the onDrop callback when a drop event is fired', () => { const dropZone = mountWithAppProvider(); - const event = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(files, files, []); }); it('calls the onDrop callback when a drop event is fired on document twice when a duplicate file is added consecutively', () => { const dropZone = mountWithAppProvider(); - const event1 = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event1); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(files, files, []); - const event2 = createEvent('drop', duplicateFiles); - dropZone.getDOMNode().dispatchEvent(event2); + fireEvent({element: dropZone, testFiles: duplicateFiles}); expect(spy).toHaveBeenCalledWith(duplicateFiles, duplicateFiles, []); }); @@ -80,8 +77,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - const event = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles); }); @@ -89,8 +85,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - const event = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(acceptedFiles); }); @@ -98,29 +93,25 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - const event = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(rejectedFiles); }); - it('calls the onDragEnter callback when a dragEnter event is fired', () => { + it('calls the onDragEnter callback when a dragenter event is fired', () => { const dropZone = mountWithAppProvider(); - const event = createEvent('dragenter', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone, eventType: 'dragenter'}); expect(spy).toHaveBeenCalled(); }); - it('calls the onDragOver callback when a dragOver event is fired', () => { + it('calls the onDragOver callback when a dragover event is fired', () => { const dropZone = mountWithAppProvider(); - const event = createEvent('dragover', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone, eventType: 'dragover'}); expect(spy).toHaveBeenCalled(); }); - it('calls the onDragLeave callback when a dragLeave event is fired', () => { + it('calls the onDragLeave callback when a dragleave event is fired', () => { const dropZone = mountWithAppProvider(); - const event = createEvent('dragleave', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone, eventType: 'dragleave'}); expect(spy).toHaveBeenCalled(); }); @@ -131,8 +122,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - const event = createEvent('drop', files); - dropZone.getDOMNode().dispatchEvent(event); + fireEvent({element: dropZone}); expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles); }); @@ -148,13 +138,13 @@ describe('', () => { onDragOver={spy} />, ); - fireEvent('drop', dropZone, spy); + fireEvent({element: dropZone, spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragenter', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragenter', spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragleave', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragleave', spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragover', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragover', spy}); expect(spy).not.toHaveBeenCalled(); }); @@ -171,17 +161,17 @@ describe('', () => { ); // Initial event to populate zone with data (should succeed) - fireEvent('drop', dropZone, spy); + fireEvent({element: dropZone, spy}); expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles); // All events should now be ignored - fireEvent('drop', dropZone, spy); + fireEvent({element: dropZone, spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragenter', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragenter', spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragleave', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragleave', spy}); expect(spy).not.toHaveBeenCalled(); - fireEvent('dragover', dropZone, spy); + fireEvent({element: dropZone, eventType: 'dragover', spy}); expect(spy).not.toHaveBeenCalled(); }); @@ -237,7 +227,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const displayText = dropZone.find(DisplayText); const caption = dropZone.find(Caption); expect(displayText).toHaveLength(0); @@ -249,7 +239,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const captionText = dropZone.find(Caption); expect(captionText.contains(overlayText)).toBe(true); }); @@ -259,7 +249,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const captionText = dropZone.find(Caption); expect(captionText.contains(overlayText)).toBe(true); }); @@ -269,7 +259,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const displayText = dropZone.find(DisplayText); expect(displayText.contains(overlayText)).toBe(true); }); @@ -282,7 +272,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const displayText = dropZone.find(DisplayText); const caption = dropZone.find(Caption); expect(displayText).toHaveLength(0); @@ -294,7 +284,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const captionText = dropZone.find(Caption); expect(captionText.contains(errorOverlayText)).toBe(true); }); @@ -304,7 +294,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const captionText = dropZone.find(Caption); expect(captionText.contains(errorOverlayText)).toBe(true); }); @@ -314,7 +304,7 @@ describe('', () => { const dropZone = mountWithAppProvider( , ); - triggerDragEnter(dropZone); + fireEvent({element: dropZone, eventType: 'dragenter'}); const displayText = dropZone.find(DisplayText); expect(displayText.contains(errorOverlayText)).toBe(true); }); @@ -363,19 +353,29 @@ function setBoundingClientRect(size: keyof typeof widths) { }); } -function triggerDragEnter(element: ReactWrapper) { - const event = createEvent('dragenter', files); - element.getDOMNode().dispatchEvent(event); - clock.tick(50); +function fireEvent({ + element, + eventType = 'drop', + testFiles = files, + spy, +}: { + element: ReactWrapper; + eventType?: string; + spy?: jest.Mock; + delay?: number; + testFiles?: Array; +}) { + if (spy) { + spy.mockReset(); + } + const event = createEvent(eventType, testFiles); + element + .find('div') + .at(3) + .getDOMNode() + .dispatchEvent(event); + if (eventType === 'dragenter') { + clock.tick(50); + } element.update(); } - -function fireEvent( - eventType: string, - element: ReactWrapper, - spy: jest.Mock, -) { - spy.mockReset(); - const event = createEvent(eventType, files); - element.getDOMNode().dispatchEvent(event); -} diff --git a/src/components/Form/Form.tsx b/src/components/Form/Form.tsx index b5ce04d6533..cdf7a78b661 100644 --- a/src/components/Form/Form.tsx +++ b/src/components/Form/Form.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import VisuallyHidden from '../VisuallyHidden'; +import {withAppProvider, WithAppProviderProps} from '../AppProvider'; export type Enctype = | 'application/x-www-form-urlencoded' @@ -36,7 +37,9 @@ export interface Props { onSubmit(event: React.FormEvent): void; } -export default class Form extends React.PureComponent { +type CombinedProps = Props & WithAppProviderProps; + +class Form extends React.PureComponent { render() { const { acceptCharset, @@ -49,12 +52,15 @@ export default class Form extends React.PureComponent { name, noValidate, target, + polaris: {intl}, } = this.props; const autoCompleteInputs = normalizeAutoComplete(autoComplete); const submitMarkup = implicitSubmit ? ( - ) : null; @@ -95,3 +101,5 @@ function normalizeAutoComplete(autoComplete?: boolean) { return autoComplete ? 'on' : 'off'; } + +export default withAppProvider()(Form); diff --git a/src/components/Layout/README.md b/src/components/Layout/README.md index b2976f9087b..f4282550a9a 100644 --- a/src/components/Layout/README.md +++ b/src/components/Layout/README.md @@ -287,8 +287,8 @@ Use to create a ½ + ½ layout. Can be used to display content of equal importan resourceName={{singular: 'product', plural: 'products'}} items={[ { - id: 341, - url: 'produdcts/341', + id: 342, + url: 'produdcts/342', name: 'Black & orange scarf', sku: '9234194023', quantity: '100', @@ -300,8 +300,8 @@ Use to create a ½ + ½ layout. Can be used to display content of equal importan ), }, { - id: 256, - url: 'produdcts/256', + id: 257, + url: 'produdcts/257', name: 'Tucan scarf', sku: '9234194010', quantity: '201', @@ -354,8 +354,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal resourceName={{singular: 'product', plural: 'products'}} items={[ { - id: 341, - url: 'produdcts/341', + id: 343, + url: 'produdcts/343', name: 'Black & orange scarf', sku: '9234194023', quantity: '254', @@ -367,8 +367,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal ), }, { - id: 256, - url: 'produdcts/256', + id: 258, + url: 'produdcts/258', name: 'Tucan scarf', sku: '9234194010', quantity: '201', @@ -412,8 +412,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal resourceName={{singular: 'product', plural: 'products'}} items={[ { - id: 341, - url: 'produdcts/341', + id: 344, + url: 'produdcts/344', name: 'Black & orange scarf', sku: '9234194023', quantity: '100', @@ -425,8 +425,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal ), }, { - id: 256, - url: 'produdcts/256', + id: 259, + url: 'produdcts/259', name: 'Tucan scarf', sku: '9234194010', quantity: '201', @@ -470,8 +470,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal resourceName={{singular: 'product', plural: 'products'}} items={[ { - id: 341, - url: 'produdcts/341', + id: 345, + url: 'produdcts/345', name: 'Black & orange scarf', sku: '9234194023', quantity: '1230', @@ -483,8 +483,8 @@ Use to create a ⅓ + ⅓ + ⅓ layout. Can be used to display content of equal ), }, { - id: 256, - url: 'produdcts/256', + id: 260, + url: 'produdcts/260', name: 'Tucan scarf', sku: '9234194010', quantity: '701', diff --git a/src/components/Modal/README.md b/src/components/Modal/README.md index 94fb51c2083..486094674aa 100644 --- a/src/components/Modal/README.md +++ b/src/components/Modal/README.md @@ -368,17 +368,17 @@ class ModalExample extends React.Component { {}} + connectedRight={ + + } /> - - - diff --git a/src/components/Modal/components/CloseButton/CloseButton.tsx b/src/components/Modal/components/CloseButton/CloseButton.tsx index 371f3fbc4e0..53f0fb5dbf6 100644 --- a/src/components/Modal/components/CloseButton/CloseButton.tsx +++ b/src/components/Modal/components/CloseButton/CloseButton.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import {MobileCancelMajorMonotone} from '@shopify/polaris-icons'; import {classNames} from '@shopify/css-utilities'; +import {withAppProvider, WithAppProviderProps} from '../../../AppProvider'; import Icon from '../../../Icon'; @@ -11,15 +12,23 @@ export interface Props { onClick(): void; } -export default function CloseButton({title = true, onClick}: Props) { +type CombinedProps = Props & WithAppProviderProps; + +function CloseButton({title = true, onClick, polaris: {intl}}: CombinedProps) { const className = classNames( styles.CloseButton, !title && styles.withoutTitle, ); return ( - ); } + +export default withAppProvider()(CloseButton); diff --git a/src/components/Navigation/components/Section/Section.tsx b/src/components/Navigation/components/Section/Section.tsx index b11dd1d88ec..bf9ea82c5a8 100644 --- a/src/components/Navigation/components/Section/Section.tsx +++ b/src/components/Navigation/components/Section/Section.tsx @@ -112,7 +112,6 @@ export default class Section extends React.Component { - {ariaLabel} ); diff --git a/src/components/OptionList/OptionList.tsx b/src/components/OptionList/OptionList.tsx index 5c30b287f64..b9978c5c3b9 100644 --- a/src/components/OptionList/OptionList.tsx +++ b/src/components/OptionList/OptionList.tsx @@ -147,7 +147,7 @@ export class OptionList extends React.Component { {titleMarkup}
      diff --git a/src/components/Page/components/Header/Header.tsx b/src/components/Page/components/Header/Header.tsx index e8e79da40fd..f459ead504a 100644 --- a/src/components/Page/components/Header/Header.tsx +++ b/src/components/Page/components/Header/Header.tsx @@ -184,7 +184,11 @@ class Header extends React.PureComponent { private renderRollupAction = () => { const {rollupOpen} = this.state; - const {secondaryActions = [], actionGroups = []} = this.props; + const { + secondaryActions = [], + actionGroups = [], + polaris: {intl}, + } = this.props; const rollupMarkup = this.hasRollup ? (
      { plain icon={HorizontalDotsMinor} onClick={this.handleRollupToggle} + accessibilityLabel={intl.translate( + 'Polaris.Page.Header.rollupButton', + )} /> } > diff --git a/src/components/Sheet/README.md b/src/components/Sheet/README.md index f9f281c3b46..31eb20c4b07 100644 --- a/src/components/Sheet/README.md +++ b/src/components/Sheet/README.md @@ -207,9 +207,11 @@ class SheetExample extends React.Component {
      diff --git a/src/components/Tabs/Tabs.tsx b/src/components/Tabs/Tabs.tsx index dbe7aeedb61..8580028228d 100644 --- a/src/components/Tabs/Tabs.tsx +++ b/src/components/Tabs/Tabs.tsx @@ -5,6 +5,7 @@ import {classNames} from '@shopify/css-utilities'; import Icon from '../Icon'; import Popover from '../Popover'; +import {withAppProvider, WithAppProviderProps} from '../AppProvider'; import {TabDescriptor} from './types'; import {getVisibleAndHiddenTabIndices} from './utilities'; @@ -25,6 +26,8 @@ export interface Props { onSelect?(selectedTabIndex: number): void; } +type CombinedProps = Props & WithAppProviderProps; + export interface State { disclosureWidth: number; tabWidths: number[]; @@ -35,7 +38,7 @@ export interface State { tabToFocus: number; } -export default class Tabs extends React.PureComponent { +class Tabs extends React.PureComponent { static Panel = Panel; static getDerivedStateFromProps(nextProps: Props, prevState: State) { @@ -66,7 +69,13 @@ export default class Tabs extends React.PureComponent { }; render() { - const {tabs, selected, fitted, children} = this.props; + const { + tabs, + selected, + fitted, + children, + polaris: {intl}, + } = this.props; const {tabToFocus, visibleTabs, hiddenTabs, showDisclosure} = this.state; const disclosureTabs = hiddenTabs.map((tabIndex) => tabs[tabIndex]); @@ -101,6 +110,7 @@ export default class Tabs extends React.PureComponent { tabIndex={-1} className={styles.DisclosureActivator} onClick={this.handleDisclosureActivatorClick} + aria-label={intl.translate('Polaris.Tabs.toggleTabsLabel')} > @@ -324,3 +334,5 @@ function handleKeyDown(event: React.KeyboardEvent) { event.stopPropagation(); } } + +export default withAppProvider()(Tabs); diff --git a/src/components/TextField/README.md b/src/components/TextField/README.md index 3c374886e58..01d38c47e8d 100644 --- a/src/components/TextField/README.md +++ b/src/components/TextField/README.md @@ -733,7 +733,7 @@ class SeparateValidationErrorExample extends React.Component { - ) : null; diff --git a/src/components/TopBar/components/SearchField/SearchField.tsx b/src/components/TopBar/components/SearchField/SearchField.tsx index f54dbf07b1d..0bf635590cf 100644 --- a/src/components/TopBar/components/SearchField/SearchField.tsx +++ b/src/components/TopBar/components/SearchField/SearchField.tsx @@ -1,12 +1,16 @@ import * as React from 'react'; +import {createUniqueIDFactory} from '@shopify/javascript-utilities/other'; import {CircleCancelMinor, SearchMinor} from '@shopify/polaris-icons'; import {classNames} from '@shopify/css-utilities'; import Icon from '../../../Icon'; +import VisuallyHidden from '../../../VisuallyHidden'; import {withAppProvider, WithAppProviderProps} from '../../../AppProvider'; import styles from './SearchField.scss'; +const getUniqueId = createUniqueIDFactory('SearchField'); + export interface Props { /** Initial value for the input */ value: string; @@ -30,6 +34,7 @@ export type ComposedProps = Props & WithAppProviderProps; export class SearchField extends React.Component { private input: React.RefObject = React.createRef(); + private searchId = getUniqueId(); componentDidMount() { const {focused} = this.props; @@ -92,7 +97,13 @@ export class SearchField extends React.Component { onFocus={this.handleFocus} onBlur={this.handleBlur} > + + +