Fixing accessibility issues in shitlist (Shopify#1565)

* Fixing accessibility issues in shitlist
Vilango · May 29, 2019 · ebda399 · ebda399
1 parent 306c7c1
commit ebda399
Show file tree

Hide file tree

Showing 28 changed files with 324 additions and 660 deletions.
diff --git 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
diff --git a/src/components/DropZone/DropZone.tsx b/src/components/DropZone/DropZone.tsx
@@ -234,6 +234,7 @@ export class DropZone extends React.Component<CombinedProps, State> {
       active,
       overlay,
       allowMultiple,
+      polaris: {intl},
     } = this.props;
 
     const inputAttributes: object = {
@@ -292,38 +293,35 @@ export class DropZone extends React.Component<CombinedProps, State> {
         </div>
       ) : null;
 
-    const dropZoneMarkup = (
-      <div
-        ref={this.node}
-        className={classes}
-        aria-disabled={disabled}
-        onClick={this.handleClick}
-        onDragStart={handleDragStart}
-      >
-        {dragOverlay}
-        {dragErrorOverlay}
-        <div className={styles.Container}>{children}</div>
-        <VisuallyHidden>
-          <input {...inputAttributes} />
-        </VisuallyHidden>
-      </div>
-    );
-
-    const labelledDropzoneMarkup = label ? (
-      <Labelled
-        id={id}
-        label={label}
-        action={labelAction}
-        labelHidden={labelHidden}
-      >
-        {dropZoneMarkup}
-      </Labelled>
-    ) : (
-      dropZoneMarkup
-    );
+    const labelValue = label
+      ? label
+      : intl.translate('Polaris.DropZone.FileUpload.label');
+    const labelHiddenValue = label ? labelHidden : true;
 
     return (
-      <Provider value={this.getContext}>{labelledDropzoneMarkup}</Provider>
+      <Provider value={this.getContext}>
+        <Labelled
+          id={id}
+          label={labelValue}
+          action={labelAction}
+          labelHidden={labelHiddenValue}
+        >
+          <div
+            ref={this.node}
+            className={classes}
+            aria-disabled={disabled}
+            onClick={this.handleClick}
+            onDragStart={handleDragStart}
+          >
+            {dragOverlay}
+            {dragErrorOverlay}
+            <div className={styles.Container}>{children}</div>
+            <VisuallyHidden>
+              <input {...inputAttributes} />
+            </VisuallyHidden>
+          </div>
+        </Labelled>
+      </Provider>
     );
   }
 

diff --git a/src/components/DropZone/tests/DropZone.test.tsx b/src/components/DropZone/tests/DropZone.test.tsx
@@ -53,19 +53,16 @@ describe('<DropZone />', () => {
 
   it('calls the onDrop callback when a drop event is fired', () => {
     const dropZone = mountWithAppProvider(<DropZone onDrop={spy} />);
-    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(<DropZone onDrop={spy} />);
-    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,47 +77,41 @@ describe('<DropZone />', () => {
     const dropZone = mountWithAppProvider(
       <DropZone onDrop={spy} accept="image/jpeg" />,
     );
-    const event = createEvent('drop', files);
-    dropZone.getDOMNode().dispatchEvent(event);
+    fireEvent({element: dropZone});
     expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles);
   });
 
   it('calls the onDropAccepted callback with acceptedFiles when it accepts only jpeg', () => {
     const dropZone = mountWithAppProvider(
       <DropZone onDropAccepted={spy} accept="image/jpeg" />,
     );
-    const event = createEvent('drop', files);
-    dropZone.getDOMNode().dispatchEvent(event);
+    fireEvent({element: dropZone});
     expect(spy).toHaveBeenCalledWith(acceptedFiles);
   });
 
   it('calls the onDropRejected callback with rejectedFiles when it accepts only jpeg', () => {
     const dropZone = mountWithAppProvider(
       <DropZone onDropRejected={spy} accept="image/jpeg" />,
     );
-    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(<DropZone onDragEnter={spy} />);
-    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(<DropZone onDragOver={spy} />);
-    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(<DropZone onDragLeave={spy} />);
-    const event = createEvent('dragleave', files);
-    dropZone.getDOMNode().dispatchEvent(event);
+    fireEvent({element: dropZone, eventType: 'dragleave'});
     expect(spy).toHaveBeenCalled();
   });
 
@@ -131,8 +122,7 @@ describe('<DropZone />', () => {
     const dropZone = mountWithAppProvider(
       <DropZone onDrop={spy} customValidator={customValidator} />,
     );
-    const event = createEvent('drop', files);
-    dropZone.getDOMNode().dispatchEvent(event);
+    fireEvent({element: dropZone});
     expect(spy).toHaveBeenCalledWith(files, acceptedFiles, rejectedFiles);
   });
 
@@ -148,13 +138,13 @@ describe('<DropZone />', () => {
         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('<DropZone />', () => {
     );
 
     // 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('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone overlayText={overlayText} />,
       );
-      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('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone overlayText={overlayText} />,
       );
-      triggerDragEnter(dropZone);
+      fireEvent({element: dropZone, eventType: 'dragenter'});
       const captionText = dropZone.find(Caption);
       expect(captionText.contains(overlayText)).toBe(true);
     });
@@ -259,7 +249,7 @@ describe('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone overlayText={overlayText} />,
       );
-      triggerDragEnter(dropZone);
+      fireEvent({element: dropZone, eventType: 'dragenter'});
       const captionText = dropZone.find(Caption);
       expect(captionText.contains(overlayText)).toBe(true);
     });
@@ -269,7 +259,7 @@ describe('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone overlayText={overlayText} />,
       );
-      triggerDragEnter(dropZone);
+      fireEvent({element: dropZone, eventType: 'dragenter'});
       const displayText = dropZone.find(DisplayText);
       expect(displayText.contains(overlayText)).toBe(true);
     });
@@ -282,7 +272,7 @@ describe('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone errorOverlayText={errorOverlayText} accept="image/gif" />,
       );
-      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('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone errorOverlayText={errorOverlayText} accept="image/gif" />,
       );
-      triggerDragEnter(dropZone);
+      fireEvent({element: dropZone, eventType: 'dragenter'});
       const captionText = dropZone.find(Caption);
       expect(captionText.contains(errorOverlayText)).toBe(true);
     });
@@ -304,7 +294,7 @@ describe('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone errorOverlayText={errorOverlayText} accept="image/gif" />,
       );
-      triggerDragEnter(dropZone);
+      fireEvent({element: dropZone, eventType: 'dragenter'});
       const captionText = dropZone.find(Caption);
       expect(captionText.contains(errorOverlayText)).toBe(true);
     });
@@ -314,7 +304,7 @@ describe('<DropZone />', () => {
       const dropZone = mountWithAppProvider(
         <DropZone errorOverlayText={errorOverlayText} accept="image/gif" />,
       );
-      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<any, any>) {
-  const event = createEvent('dragenter', files);
-  element.getDOMNode().dispatchEvent(event);
-  clock.tick(50);
+function fireEvent({
+  element,
+  eventType = 'drop',
+  testFiles = files,
+  spy,
+}: {
+  element: ReactWrapper<any, any>;
+  eventType?: string;
+  spy?: jest.Mock;
+  delay?: number;
+  testFiles?: Array<Object>;
+}) {
+  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<any, any>,
-  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
@@ -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<HTMLFormElement>): void;
 }
 
-export default class Form extends React.PureComponent<Props> {
+type CombinedProps = Props & WithAppProviderProps;
+
+class Form extends React.PureComponent<CombinedProps, never> {
   render() {
     const {
       acceptCharset,
@@ -49,12 +52,15 @@ export default class Form extends React.PureComponent<Props> {
       name,
       noValidate,
       target,
+      polaris: {intl},
     } = this.props;
     const autoCompleteInputs = normalizeAutoComplete(autoComplete);
 
     const submitMarkup = implicitSubmit ? (
       <VisuallyHidden>
-        <button type="submit" aria-hidden="true" />
+        <button type="submit" aria-hidden="true">
+          {intl.translate('Polaris.Common.submit')}
+        </button>
       </VisuallyHidden>
     ) : null;
 
@@ -95,3 +101,5 @@ function normalizeAutoComplete(autoComplete?: boolean) {
 
   return autoComplete ? 'on' : 'off';
 }
+
+export default withAppProvider<Props>()(Form);