🪟 🔧 Add accessibility linting (#15859)

* turn on recommended rules * remove autofocus prop (form already autofocuses there without it anyways) * remove autofocus prop from connector request form (field autofocuses without it anyways) * remove autofocus from popout menu (doesn't autofocus even with it on) * Sidebar popout menu accessibility (role, tabindex, onfocus) * switch to use onFocus vs onClick * edit connection name button should be a real button * maybe create element instead for connectionname? * fix weird ts button things * modal and switch * add label control rule * disable for tooltip temporarily * fix rebase issues * make navbar popout buttons actual buttons * fix button appearance in sidebar * remove border from path popout button * review cleanup * Update airbyte-webapp/src/components/Modal/Modal.tsx Co-authored-by: Krishna Glick <krishna@airbyte.io> * leave modal default for now * missing comment space * make eslint jsx a11y plugin a dev dependency * fix stylelint problem * add mock intersection observer * review changes * switch component * move the event bubbling to the StatusCell * Remove autoFocus * order z-indices by value * missing bracket Co-authored-by: Krishna Glick <krishna@airbyte.io> Co-authored-by: Tim Roes <tim@airbyte.io>
airbytehq · Sep 7, 2022 · 42c5265 · 42c5265
1 parent fbeb6a4
commit 42c5265
Show file tree

Hide file tree

Showing 22 changed files with 365 additions and 242 deletions.
diff --git a/airbyte-webapp/.eslintrc b/airbyte-webapp/.eslintrc
@@ -5,9 +5,10 @@
     "plugin:jest/recommended",
     "prettier",
     "plugin:prettier/recommended",
-    "plugin:css-modules/recommended"
+    "plugin:css-modules/recommended",
+    "plugin:jsx-a11y/recommended"
   ],
-  "plugins": ["react", "@typescript-eslint", "prettier", "unused-imports", "css-modules"],
+  "plugins": ["react", "@typescript-eslint", "prettier", "unused-imports", "css-modules", "jsx-a11y"],
   "parserOptions": {
     "ecmaVersion": 2020,
     "sourceType": "module",
@@ -16,6 +17,7 @@
     }
   },
   "rules": {
+    "jsx-a11y/label-has-associated-control": "error",
     "curly": "warn",
     "css-modules/no-undef-class": ["error", { "camelCase": true }],
     "css-modules/no-unused-class": ["error", { "camelCase": true }],

diff --git a/airbyte-webapp/package-lock.json b/airbyte-webapp/package-lock.json
diff --git a/airbyte-webapp/package.json b/airbyte-webapp/package.json
@@ -107,6 +107,7 @@
     "eslint-config-react-app": "^7.0.1",
     "eslint-plugin-css-modules": "^2.11.0",
     "eslint-plugin-jest": "^26.5.3",
+    "eslint-plugin-jsx-a11y": "^6.6.1",
     "eslint-plugin-prettier": "^4.0.0",
     "eslint-plugin-unused-imports": "^2.0.0",
     "express": "^4.18.1",

diff --git a/airbyte-webapp/src/components/EntityTable/components/StatusCell.tsx b/airbyte-webapp/src/components/EntityTable/components/StatusCell.tsx
@@ -35,7 +35,16 @@ const StatusCell: React.FC<IProps> = ({ enabled, isManual, id, onChangeStatus, i
       onChangeStatus(id);
     };
 
-    return <Switch checked={enabled} onChange={onSwitchChange} disabled={!allowSync} />;
+    return (
+      // this is so we can stop event propagation so the row doesn't receive the click and redirect
+      // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+      <div
+        onClick={(event: React.SyntheticEvent) => event.stopPropagation()}
+        onKeyPress={(event: React.SyntheticEvent) => event.stopPropagation()}
+      >
+        <Switch checked={enabled} onChange={onSwitchChange} disabled={!allowSync} />
+      </div>
+    );
   }
 
   if (isSyncing) {

diff --git a/airbyte-webapp/src/components/Modal/Modal.module.scss b/airbyte-webapp/src/components/Modal/Modal.module.scss
@@ -1,29 +1,47 @@
 @use "../../scss/colors";
 @use "../../scss/variables";
+@use "../../scss/z-indices";
 
 @keyframes fade-in {
   from {
     opacity: 0;
   }
 }
 
-.modal {
-  animation: fade-in 0.2s ease-out;
-  position: absolute;
+.backdrop {
+  position: fixed;
   top: 0;
   left: 0;
-  width: 100vw;
-  height: 100vh;
+  right: 0;
+  bottom: 0;
   background: rgba(colors.$black, 0.5);
+}
+
+.modalPageContainer {
+  position: absolute;
+  z-index: z-indices.$modal;
+}
+
+.modalContainer {
+  position: fixed;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  animation: fadeIn 0.2s ease-out;
   display: flex;
   justify-content: center;
   align-items: center;
-  z-index: 100;
+}
+
+.modalPanel {
+  margin-left: auto;
+  margin-right: auto;
 }
 
 .card {
   margin-left: variables.$width-size-menu;
-  max-width: calc(100% - (#{variables.$width-size-menu} - #{variables.$spacing-lg}) * 2);
+  max-width: calc(100vw - variables.$width-size-menu - variables.$spacing-lg * 2);
 
   &.sm {
     width: variables.$width-modal-sm;

diff --git a/airbyte-webapp/src/components/Modal/Modal.tsx b/airbyte-webapp/src/components/Modal/Modal.tsx
@@ -1,6 +1,6 @@
+import { Dialog } from "@headlessui/react";
 import classNames from "classnames";
-import React, { useEffect, useCallback } from "react";
-import { createPortal } from "react-dom";
+import React, { useState } from "react";
 
 import ContentCard from "components/ContentCard";
 
@@ -22,43 +22,29 @@ const cardStyleBySize = {
   xl: styles.xl,
 };
 
-const Modal: React.FC<ModalProps> = ({ children, title, onClose, clear, closeOnBackground, size, testId }) => {
-  const handleUserKeyPress = useCallback((event: KeyboardEvent, closeModal: () => void) => {
-    const { key } = event;
-    // Escape key
-    if (key === "Escape") {
-      closeModal();
-    }
-  }, []);
-
-  useEffect(() => {
-    if (!onClose) {
-      return;
-    }
-
-    const onKeyDown = (event: KeyboardEvent) => handleUserKeyPress(event, onClose);
-    window.addEventListener("keydown", onKeyDown);
-
-    return () => {
-      window.removeEventListener("keydown", onKeyDown);
-    };
-  }, [handleUserKeyPress, onClose]);
-
-  return createPortal(
-    <div
-      className={styles.modal}
-      onClick={() => (closeOnBackground && onClose ? onClose() : null)}
-      data-testid={testId}
-    >
-      {clear ? (
-        children
-      ) : (
-        <ContentCard title={title} className={classNames(styles.card, size ? cardStyleBySize[size] : undefined)}>
-          {children}
-        </ContentCard>
-      )}
-    </div>,
-    document.body
+const Modal: React.FC<ModalProps> = ({ children, title, size, onClose, clear, testId }) => {
+  const [isOpen, setIsOpen] = useState(true);
+
+  const onModalClose = () => {
+    setIsOpen(false);
+    onClose?.();
+  };
+
+  return (
+    <Dialog open={isOpen} onClose={onModalClose} data-testid={testId} className={styles.modalPageContainer}>
+      <div className={styles.backdrop} />
+      <div className={styles.modalContainer}>
+        <Dialog.Panel className={styles.modalPanel}>
+          {clear ? (
+            children
+          ) : (
+            <ContentCard title={title} className={classNames(styles.card, size ? cardStyleBySize[size] : undefined)}>
+              {children}
+            </ContentCard>
+          )}
+        </Dialog.Panel>
+      </div>
+    </Dialog>
   );
 };
 

diff --git a/airbyte-webapp/src/components/base/Popout/Popout.tsx b/airbyte-webapp/src/components/base/Popout/Popout.tsx
@@ -66,7 +66,6 @@ const Popout: React.FC<PopoutProps> = ({ onChange, targetComponent, ...props })
           targetComponent,
           onOpen: toggleOpen,
         }}
-        autoFocus
         backspaceRemovesValue={false}
         controlShouldRenderValue={false}
         hideSelectedOptions={false}

diff --git a/airbyte-webapp/src/components/base/Switch/Switch.tsx b/airbyte-webapp/src/components/base/Switch/Switch.tsx
@@ -19,7 +19,7 @@ export const Switch: React.FC<SwitchProps> = ({ loading, small, checked, value,
   });
 
   return (
-    <label onClick={(event: React.SyntheticEvent) => event.stopPropagation()} className={labelStyle}>
+    <label className={labelStyle}>
       <input
         {...props}
         className={styles.switchInput}

diff --git a/airbyte-webapp/src/components/base/Tooltip/Tooltip.tsx b/airbyte-webapp/src/components/base/Tooltip/Tooltip.tsx
@@ -1,3 +1,4 @@
+/* eslint-disable jsx-a11y/mouse-events-have-key-events */
 import { autoUpdate, flip, offset, shift, useFloating, UseFloatingProps } from "@floating-ui/react-dom";
 import classNames from "classnames";
 import React, { useState, useEffect } from "react";

diff --git a/airbyte-webapp/src/hooks/services/Modal/ModalService.test.tsx b/airbyte-webapp/src/hooks/services/Modal/ModalService.test.tsx
@@ -2,6 +2,8 @@ import { render, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { useEffectOnce } from "react-use";
 
+import { useMockIntersectionObserver } from "utils/testutils";
+
 import { ModalServiceProvider, useModalService } from "./ModalService";
 import { ModalResult } from "./types";
 
@@ -37,6 +39,10 @@ const renderModal = (resultCallback?: (reason: unknown) => void) => {
 };
 
 describe("ModalService", () => {
+  beforeEach(() => {
+    // IntersectionObserver isn't available in test environment but is used by headless-ui dialog
+    useMockIntersectionObserver();
+  });
   it("should open a modal on openModal", () => {
     const rendered = renderModal();
 

diff --git a/airbyte-webapp/src/packages/cloud/views/layout/SideBar/SideBar.tsx b/airbyte-webapp/src/packages/cloud/views/layout/SideBar/SideBar.tsx
@@ -135,12 +135,12 @@ const SideBar: React.FC = () => {
         <li>
           <SidebarPopout options={[{ value: "docs" }, { value: "slack" }, { value: "status" }, { value: "recipes" }]}>
             {({ onOpen, isOpen }) => (
-              <div className={getPopoutStyles(isOpen)} onClick={onOpen}>
+              <button className={getPopoutStyles(isOpen)} onClick={onOpen}>
                 <DocsIcon />
                 <Text>
                   <FormattedMessage id="sidebar.resources" />
                 </Text>
-              </div>
+              </button>
             )}
           </SidebarPopout>
         </li>
@@ -162,12 +162,12 @@ const SideBar: React.FC = () => {
             ]}
           >
             {({ onOpen, isOpen }) => (
-              <div className={getPopoutStyles(isOpen)} onClick={onOpen}>
+              <button className={getPopoutStyles(isOpen)} onClick={onOpen} role="menu">
                 <FontAwesomeIcon icon={faQuestionCircle} size="2x" />
                 <Text>
                   <FormattedMessage id="sidebar.support" />
                 </Text>
-              </div>
+              </button>
             )}
           </SidebarPopout>
         </li>

diff --git a/...app/src/packages/cloud/views/workspaces/WorkspacesPage/components/CreateWorkspaceForm.tsx b/...app/src/packages/cloud/views/workspaces/WorkspacesPage/components/CreateWorkspaceForm.tsx
@@ -49,7 +49,7 @@ const CreateWorkspaceForm: React.FC<CreateWorkspaceFormProps> = ({ onSubmit }) =
         <CreationForm>
           <Field name="name">
             {({ field, meta }: FieldProps<string>) => (
-              <ClearInput {...field} autoFocus type="text" error={!!meta.error && meta.touched} />
+              <ClearInput {...field} type="text" error={!!meta.error && meta.touched} />
             )}
           </Field>
           <LoadingButton type="submit" isLoading={isSubmitting} data-testid="workspaces.create">

diff --git a/...te-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/components/ConnectionName.tsx b/...te-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/components/ConnectionName.tsx
@@ -79,17 +79,16 @@ const ConnectionName: React.FC<ConnectionNameProps> = ({ connection }) => {
               onEscape={onEscape}
               onEnter={onEnter}
               disabled={loading}
-              autoFocus
             />
           </div>
         </div>
       ) : (
-        <div className={styles.nameContainer} onClick={() => setEditingState(true)}>
+        <button className={styles.nameContainer} onClick={() => setEditingState(true)}>
           <div>
             <h2>{name}</h2>
           </div>
           <FontAwesomeIcon className={styles.icon} icon={faPenToSquare} />
-        </div>
+        </button>
       )}
     </div>
   );

diff --git a/airbyte-webapp/src/pages/OnboardingPage/OnboardingPage.tsx b/airbyte-webapp/src/pages/OnboardingPage/OnboardingPage.tsx
@@ -37,7 +37,6 @@ const Content = styled.div<{ big?: boolean; medium?: boolean }>`
   align-items: center;
   min-height: 100%;
   position: relative;
-  z-index: 2;
 `;
 
 const Footer = styled.div`

diff --git a/airbyte-webapp/src/scss/_z-indices.scss b/airbyte-webapp/src/scss/_z-indices.scss
@@ -1,2 +1,4 @@
+$tooltip: 9999 + 2;
+$modal: 9999 + 1;
 $sidebar: 9999;
-$tooltip: 9999 + 1;
+$panelSplitter: 0;
diff --git a/airbyte-webapp/src/utils/testutils.tsx b/airbyte-webapp/src/utils/testutils.tsx
@@ -61,6 +61,17 @@ export const TestWrapper: React.FC = ({ children }) => (
   </ThemeProvider>
 );
 
+export const useMockIntersectionObserver = () => {
+  // IntersectionObserver isn't available in test environment but is used by the dialog component
+  const mockIntersectionObserver = jest.fn();
+  mockIntersectionObserver.mockReturnValue({
+    observe: jest.fn().mockReturnValue(null),
+    unobserve: jest.fn().mockReturnValue(null),
+    disconnect: jest.fn().mockReturnValue(null),
+  });
+  window.IntersectionObserver = mockIntersectionObserver;
+};
+
 export const mockSource: SourceRead = {
   sourceId: "test-source",
   name: "test source",

diff --git a/airbyte-webapp/src/views/Connection/CatalogTree/components/PathPopoutButton.module.scss b/airbyte-webapp/src/views/Connection/CatalogTree/components/PathPopoutButton.module.scss
@@ -15,6 +15,7 @@
   padding: 8px;
   border-radius: variables.$border-radius-xs;
   background-color: colors.$grey-100;
+  border: none;
   cursor: pointer;
 
   &:hover {

diff --git a/airbyte-webapp/src/views/Connection/CatalogTree/components/PathPopoutButton.tsx b/airbyte-webapp/src/views/Connection/CatalogTree/components/PathPopoutButton.tsx
@@ -15,12 +15,12 @@ interface PathPopoutButtonProps {
 export const PathPopoutButton: React.FC<PathPopoutButtonProps> = ({ items = [], onClick, children }) => (
   <Tooltip
     control={
-      <div className={styles.button} role="button" onClick={onClick}>
+      <button className={styles.button} onClick={onClick}>
         <Text size="sm" className={styles.text}>
           {children}
         </Text>
         <FontAwesomeIcon className={styles.arrow} icon={faSortDown} />
-      </div>
+      </button>
     }
     placement="bottom-start"
     disabled={items.length === 0}

diff --git a/airbyte-webapp/src/views/Connector/RequestConnectorModal/components/ConnectorForm.tsx b/airbyte-webapp/src/views/Connector/RequestConnectorModal/components/ConnectorForm.tsx
@@ -100,7 +100,7 @@ const ConnectorForm: React.FC<ConnectorFormProps> = ({ onSubmit, onCancel, curre
                   )
                 }
               >
-                <Input {...field} autoFocus error={!!meta.error && meta.touched} type="text" />
+                <Input {...field} error={!!meta.error && meta.touched} type="text" />
               </ControlLabelsWithMargin>
             )}
           </Field>

diff --git a/airbyte-webapp/src/views/Connector/ServiceForm/ServiceForm.test.tsx b/airbyte-webapp/src/views/Connector/ServiceForm/ServiceForm.test.tsx
@@ -5,7 +5,7 @@ import React from "react";
 import selectEvent from "react-select-event";
 
 import { AirbyteJSONSchema } from "core/jsonSchema";
-import { render } from "utils/testutils";
+import { render, useMockIntersectionObserver } from "utils/testutils";
 import { ServiceForm } from "views/Connector/ServiceForm";
 
 import { DestinationDefinitionSpecificationRead } from "../../../core/request/AirbyteClient";
@@ -353,6 +353,10 @@ describe("Service Form", () => {
     });
 
     test("should fill right values in array of objects field", async () => {
+      // IntersectionObserver isn't available in test environment but is used by headless-ui dialog
+      // used for this component
+      useMockIntersectionObserver();
+
       const addPriceListItem = useAddPriceListItem(container);
       await addPriceListItem("test-1", "1");
       await addPriceListItem("test-2", "2");

diff --git a/airbyte-webapp/src/views/layout/SideBar/SideBar.module.scss b/airbyte-webapp/src/views/layout/SideBar/SideBar.module.scss
@@ -2,6 +2,9 @@
 @use "../../../scss/variables";
 
 .menuItem {
+  background-color: transparent;
+  appearance: none;
+  border: none;
   color: colors.$grey-30;
   width: 100%;
   cursor: pointer;

diff --git a/airbyte-webapp/src/views/layout/SideBar/SideBar.tsx b/airbyte-webapp/src/views/layout/SideBar/SideBar.tsx
@@ -129,12 +129,12 @@ const SideBar: React.FC = () => {
         <li>
           <SidebarPopout options={[{ value: "docs" }, { value: "slack" }, { value: "recipes" }]}>
             {({ onOpen, isOpen }) => (
-              <div className={getPopoutStyles(isOpen)} onClick={onOpen}>
+              <button className={getPopoutStyles(isOpen)} onClick={onOpen}>
                 <DocsIcon />
                 <Text>
                   <FormattedMessage id="sidebar.resources" />
                 </Text>
-              </div>
+              </button>
             )}
           </SidebarPopout>
         </li>