refactor(settings): ui component followups

Because:

- There were still a few outstanding items to address as part of #6206

This commit:

- Eliminates all component `readerText` props. We determined they didn't add a substantial amount of value, and any accompanying a11y content can be implemented on a case-by-case basis.
- As a result of removing readerText I also refactored the the usage of component IDs and was able to eliminate the UUID dependency.
- Makes `DataBlock` only copyable. We currently don't have a need for a read-only version.
- Further cleans up some leftover event handlers and weird syntax in the new components.

packages/fxa-settings/package.json

@@ -43,8 +43,7 @@
     "react-dom": "^16.13.1",
     "react-scripts": "^3.4.1",
     "subscriptions-transport-ws": "^0.9.11",
-    "typescript": "3.9.7",
-    "uuid": "^8.3.0"
+    "typescript": "3.9.7"
   },
   "devDependencies": {
     "@babel/core": "^7.10.3",
@@ -65,7 +64,6 @@
     "@types/node": "^12.12.35",
     "@types/react": "^16.9.34",
     "@types/react-dom": "^16.9.6",
-    "@types/uuid": "^7.0.2",
     "@types/webpack": "4.41.16",
     "babel-loader": "^8.0.0",
     "eslint": "^6.8.0",

packages/fxa-settings/src/components/Checkbox/index.stories.tsx

@@ -12,10 +12,7 @@ storiesOf('Components|Checkbox', module).add('default', () => (
       <Checkbox />
     </div>
     <div className="mb-3">
-      <Checkbox
-        label="Howdy I'm a label"
-        readerText="This is additional information that screen readers can describe"
-      />
+      <Checkbox label="Howdy I'm a label" />
     </div>
     <div className="mb-3">
       <Checkbox label="Hey hey, I'm checked baby" defaultChecked />
@@ -32,7 +29,7 @@ storiesOf('Components|Checkbox', module).add('default', () => (
     <div className="mb-3">
       <Checkbox
         label="Checked and disabled? Cool flex."
-        defaultChecked={true}
+        defaultChecked
         disabled
       />
     </div>

packages/fxa-settings/src/components/Checkbox/index.test.tsx

@@ -33,10 +33,3 @@ it('can have a label', () => {
   expect(screen.getByTestId('checkbox-label')).toBeInTheDocument();
   expect(screen.getByTestId('checkbox-label')).toHaveTextContent(label);
 });
-
-it('can have screen reader text', () => {
-  const readerText = 'Sleeper 1972';
-  render(<Checkbox {...{ readerText }} />);
-  expect(screen.getByTestId('checkbox-srtext')).toBeInTheDocument();
-  expect(screen.getByTestId('checkbox-srtext')).toHaveTextContent(readerText);
-});

packages/fxa-settings/src/components/Checkbox/index.tsx

@@ -2,70 +2,36 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import React, {
-  FocusEvent,
-  MouseEvent,
-  useEffect,
-  useState,
-  useCallback,
-  ChangeEvent,
-} from 'react';
-import { v4 as uuidv4 } from 'uuid';
+import React, { MouseEvent, useState, useCallback, ChangeEvent } from 'react';
 import { ReactComponent as Checkmark } from './checkmark.svg';
 
 export type CheckboxProps = {
   defaultChecked?: boolean;
   disabled?: boolean;
-  id?: string;
   label?: string;
   onClick?: (event: MouseEvent<HTMLInputElement>) => void;
-  onBlur?: (event: FocusEvent<HTMLInputElement>) => void;
-  onFocus?: (event: FocusEvent<HTMLInputElement>) => void;
-  onChange?: (event: ChangeEvent<HTMLInputElement>) => void;
-  readerText?: string;
 };
 
 export const Checkbox = ({
   defaultChecked,
   disabled,
-  id,
   label,
-  onClick,
-  onBlur,
-  onFocus,
-  onChange,
-  readerText,
 }: CheckboxProps) => {
   const [focussed, setFocussed] = useState<boolean>(false);
   const [checked, setChecked] = useState<boolean>(defaultChecked === true);
   const [hovered, setHovered] = useState<boolean>(false);
-  const [checkboxId, setCheckboxId] = useState<string | undefined>(id);
-  const readerId = `${checkboxId}-sr`;
 
-  useEffect(() => {
-    !checkboxId && setCheckboxId(uuidv4());
-  }, [checkboxId, id]);
+  const checkboxFocus = useCallback(() => {
+    setFocussed(true);
+  }, [focussed]);
 
-  const checkboxFocus = useCallback(
-    (event: FocusEvent<HTMLInputElement>) => {
-      setFocussed(true);
-      onFocus && onFocus(event);
-    },
-    [focussed]
-  );
-
-  const checkboxBlur = useCallback(
-    (event: FocusEvent<HTMLInputElement>) => {
-      setFocussed(false);
-      onBlur && onBlur(event);
-    },
-    [focussed]
-  );
+  const checkboxBlur = useCallback(() => {
+    setFocussed(false);
+  }, [focussed]);
 
   const checkboxChange = useCallback(
     (event: ChangeEvent<HTMLInputElement>) => {
       setChecked(event.target.checked);
-      onChange && onChange(event);
     },
     [checked]
   );
@@ -85,8 +51,6 @@ export const Checkbox = ({
         <input
           className="absolute inset-0 w-full h-full opacity-0 cursor-pointer"
           type="checkbox"
-          id={checkboxId}
-          aria-describedby={readerText ? readerId : undefined}
           onFocus={checkboxFocus}
           onBlur={checkboxBlur}
           onChange={checkboxChange}
@@ -94,7 +58,6 @@ export const Checkbox = ({
           {...{
             defaultChecked,
             disabled,
-            onClick,
           }}
         />
         <span
@@ -131,11 +94,6 @@ export const Checkbox = ({
           {label}
         </span>
       )}
-      {readerText && (
-        <span data-testid="checkbox-srtext" className="sr-only" id={readerId}>
-          {readerText}
-        </span>
-      )}
     </label>
   );
 };

packages/fxa-settings/src/components/DataBlock/index.stories.tsx

@@ -9,48 +9,22 @@ import DataBlock from './index';
 storiesOf('Components|DataBlock', module)
   .add('single', () => (
     <div className="p-10 max-w-lg">
-      <div className="mb-3">
-        <DataBlock value="ANMD 1S09 7Y2Y 4EES 02CW BJ6Z PYKP H69F" />
-        <small className="block mt-2">Normal</small>
-      </div>
-      <div className="mb-3">
-        <DataBlock value="ANMD 1S09 7Y2Y 4EES 02CW BJ6Z PYKP H69F" copyable />
-        <small className="block mt-2">Copyable</small>
-      </div>
+      <DataBlock value="ANMD 1S09 7Y2Y 4EES 02CW BJ6Z PYKP H69F" />
     </div>
   ))
   .add('multiple', () => (
     <div className="p-10 max-w-sm">
-      <div className="mb-3">
-        <DataBlock
-          value={[
-            'C1OFZW7R04',
-            'XVKRLKERT4',
-            'CF0V94X204',
-            'C3THX2SGZ4',
-            'UXC6NRQT54',
-            '24RF9WFA44',
-            'ZBULPFN7J4',
-            'D4J6KY8FL4',
-          ]}
-        />
-        <small className="block mt-2">Normal</small>
-      </div>
-      <div className="mb-3">
-        <DataBlock
-          value={[
-            'C1OFZW7R04',
-            'XVKRLKERT4',
-            'CF0V94X204',
-            'C3THX2SGZ4',
-            'UXC6NRQT54',
-            '24RF9WFA44',
-            'ZBULPFN7J4',
-            'D4J6KY8FL4',
-          ]}
-          copyable
-        />
-        <small className="block mt-2">Copyable</small>
-      </div>
+      <DataBlock
+        value={[
+          'C1OFZW7R04',
+          'XVKRLKERT4',
+          'CF0V94X204',
+          'C3THX2SGZ4',
+          'UXC6NRQT54',
+          '24RF9WFA44',
+          'ZBULPFN7J4',
+          'D4J6KY8FL4',
+        ]}
+      />
     </div>
   ));

packages/fxa-settings/src/components/DataBlock/index.test.tsx

@@ -36,7 +36,7 @@ it('can render multiple values', () => {
 });
 
 it('can copy a single value to the clipboard', () => {
-  render(<DataBlock value={singleValue} copyable />);
+  render(<DataBlock value={singleValue} />);
   fireEvent.click(screen.getByTestId('datablock-button'));
   expect(window.navigator.clipboard.writeText).toHaveBeenCalledWith(
     singleValue
@@ -44,21 +44,9 @@ it('can copy a single value to the clipboard', () => {
 });
 
 it('can copy multiple values to the clipboard', () => {
-  render(<DataBlock value={multiValue} copyable />);
+  render(<DataBlock value={multiValue} />);
   fireEvent.click(screen.getByTestId('datablock-button'));
   expect(window.navigator.clipboard.writeText).toHaveBeenCalledWith(
     multiValue.join(', ')
   );
 });
-
-it('can be a read-only (disabled) element', () => {
-  render(<DataBlock value={singleValue} />);
-  expect(screen.getByTestId('datablock-button')).toBeDisabled();
-});
-
-it('can have screen reader text', () => {
-  const readerText = 'Born Of You';
-  render(<DataBlock {...{ readerText }} value={singleValue} />);
-  expect(screen.getByTestId('datablock-srtext')).toBeInTheDocument();
-  expect(screen.getByTestId('datablock-srtext')).toHaveTextContent(readerText);
-});

packages/fxa-settings/src/components/DataBlock/index.tsx

@@ -2,37 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import React, {
-  useCallback,
-  useState,
-  useEffect,
-  MouseEvent,
-  FocusEvent,
-} from 'react';
-import { v4 as uuidv4 } from 'uuid';
+import React from 'react';
 import { copy } from '../../lib/clipboard';
 
 export type DataBlockProps = {
   value: string | string[];
-  copyable?: boolean;
-  id?: string;
   onCopied?: (copiedValue: string) => void;
-  onClick?: (event: MouseEvent<HTMLButtonElement>) => void;
-  onBlur?: (event: FocusEvent<HTMLButtonElement>) => void;
-  onFocus?: (event: FocusEvent<HTMLButtonElement>) => void;
-  readerText?: string;
 };
 
-export const DataBlock = ({
-  value,
-  id,
-  copyable,
-  onCopied,
-  readerText,
-}: DataBlockProps) => {
+export const DataBlock = ({ value, onCopied }: DataBlockProps) => {
   const valueIsArray = Array.isArray(value);
-  const [inputId, setInputId] = useState<string | undefined>(id);
-  const readerId = `${inputId}-sr`;
 
   async function copyToClipboard() {
     const copyValue = valueIsArray
@@ -43,46 +22,24 @@ export const DataBlock = ({
     onCopied && onCopied(copyValue);
   }
 
-  useEffect(() => {
-    !inputId && setInputId(uuidv4());
-  }, [inputId, id]);
-
-  const onClick = (event: MouseEvent<HTMLButtonElement>) => {
-    copyable && copyToClipboard();
-  };
-
   return (
-    <>
-      <button
-        className={`flex rounded-xl px-7 font-mono text-sm text-green-900 bg-green-800 bg-opacity-10 flex-wrap ${
-          valueIsArray ? 'py-4' : 'py-5'
-        } ${
-          copyable
-            ? 'hover:bg-opacity-20 focus:bg-opacity-30 active:bg-opacity-30'
-            : 'cursor-default select-text'
-        }`}
-        aria-describedby={readerText ? readerId : undefined}
-        id={inputId}
-        data-testid="datablock-button"
-        disabled={!copyable}
-        {...{ onClick }}
-      >
-        {valueIsArray ? (
-          (value as string[]).map((item) => (
-            <span key={item} className="flex-50% py-1">
-              {item}
-            </span>
-          ))
-        ) : (
-          <span>{value}</span>
-        )}
-      </button>
-      {readerText && (
-        <span data-testid="datablock-srtext" className="sr-only" id={readerId}>
-          {readerText}
-        </span>
+    <button
+      className={`flex rounded-xl px-7 font-mono text-sm text-green-900 bg-green-800 bg-opacity-10 flex-wrap hover:bg-opacity-20 focus:bg-opacity-30 active:bg-opacity-30 ${
+        valueIsArray ? 'py-4' : 'py-5'
+      }`}
+      data-testid="datablock-button"
+      onClick={copyToClipboard}
+    >
+      {valueIsArray ? (
+        (value as string[]).map((item) => (
+          <span key={item} className="flex-50% py-1">
+            {item}
+          </span>
+        ))
+      ) : (
+        <span>{value}</span>
       )}
-    </>
+    </button>
   );
 };
 

packages/fxa-settings/src/components/PasswordInput/index.stories.tsx

@@ -8,8 +8,6 @@ import PasswordInput from './index';
 
 storiesOf('Components|PasswordInput', module).add('default', () => (
   <div className="p-10 max-w-lg">
-    <div className="mb-3">
-      <PasswordInput label="You think you know how to password? Enter it here." />
-    </div>
+    <PasswordInput label="You think you know how to password? Enter it here." />
   </div>
 ));

packages/fxa-settings/src/components/PasswordInput/index.tsx

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import React, { useState, useCallback, ChangeEvent, FocusEvent } from 'react';
+import React, { useState, useCallback, ChangeEvent } from 'react';
 import TextInput, { TextInputProps } from '../TextInput';
 import { ReactComponent as OpenEye } from './eye-open.svg';
 import { ReactComponent as ClosedEye } from './eye-closed.svg';
@@ -12,7 +12,6 @@ type PasswordInputProps = Omit<TextInputProps, 'type'>;
 export const PasswordInput = ({
   defaultValue,
   disabled,
-  id,
   label,
   placeholder,
 }: PasswordInputProps) => {
@@ -32,7 +31,6 @@ export const PasswordInput = ({
       {...{
         defaultValue,
         disabled,
-        id,
         label,
         placeholder,
         onChange,

packages/fxa-settings/src/components/TextInput/index.stories.tsx

@@ -14,7 +14,6 @@ storiesOf('Components|TextInput', module)
           label="Default label"
           placeholder="Here's a suggestion"
           type="text"
-          readerText="This is additional information that screen readers can describe"
         />
       </div>
       <div className="mb-3">

packages/fxa-settings/src/components/TextInput/index.test.tsx

@@ -43,27 +43,7 @@ it('accepts various input types', () => {
   });
 });
 
-it('can have screen reader text', () => {
-  const readerText = 'Never Really Been Another Way Out';
-  render(<TextInput {...{ label, readerText }} />);
-  expect(screen.getByTestId('input-srtext')).toBeInTheDocument();
-  expect(screen.getByTestId('input-srtext')).toHaveTextContent(readerText);
-});
-
-it('accepts a custom id', () => {
-  const id = 'h6fcK_fRYaI';
-  render(<TextInput {...{ label, id }} />);
-  expect(screen.getByTestId('input-field')).toHaveAttribute('id', id);
-  expect(screen.getByTestId('input-label')).toHaveAttribute('for', id);
-});
-
-it('creates a random id if one is not supplied', () => {
-  render(<TextInput {...{ label }} />);
-  const id = screen.getByTestId('input-field').getAttribute('id');
-  expect(screen.getByTestId('input-label')).toHaveAttribute('for', id);
-});
-
-it('can have extra content', () => {
+it('can render adjacent children', () => {
   render(
     <TextInput {...{ label }}>
       <p data-testid="input-children">Hey</p>

packages/fxa-settings/src/components/TextInput/index.tsx

@@ -2,25 +2,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import React, {
-  KeyboardEvent,
-  FocusEvent,
-  ChangeEvent,
-  useState,
-  useCallback,
-  useEffect,
-  ReactElement,
-} from 'react';
-import { v4 as uuidv4 } from 'uuid';
+import React, { ChangeEvent, useState, useCallback, ReactElement } from 'react';
 
 export type TextInputProps = {
   defaultValue?: string | number;
   disabled?: boolean;
   children?: ReactElement;
-  id?: string;
   label: string;
   placeholder?: string;
-  readerText?: string;
   onChange?: (event: ChangeEvent<HTMLInputElement>) => void;
   type?: 'text' | 'email' | 'tel' | 'number' | 'url' | 'password';
 };
@@ -29,35 +18,21 @@ export const TextInput = ({
   defaultValue,
   disabled,
   children,
-  id,
   label,
   placeholder,
   onChange,
-  readerText,
   type = 'text',
 }: TextInputProps) => {
   const [focussed, setFocussed] = useState<boolean>(false);
   const [hasContent, setHasContent] = useState<boolean>(defaultValue != null);
-  const [inputId, setInputId] = useState<string | undefined>(id);
-  const readerId = `${inputId}-sr`;
 
-  useEffect(() => {
-    !inputId && setInputId(uuidv4());
-  }, [inputId, id]);
+  const onFocus = useCallback(() => {
+    setFocussed(true);
+  }, [focussed]);
 
-  const onFocus = useCallback(
-    (event: FocusEvent<HTMLInputElement>) => {
-      setFocussed(true);
-    },
-    [focussed]
-  );
-
-  const onBlur = useCallback(
-    (event: FocusEvent<HTMLInputElement>) => {
-      setFocussed(false);
-    },
-    [focussed]
-  );
+  const onBlur = useCallback(() => {
+    setFocussed(false);
+  }, [focussed]);
 
   const textFieldChange = useCallback(
     (event: ChangeEvent<HTMLInputElement>) => {
@@ -68,15 +43,14 @@ export const TextInput = ({
   );
 
   return (
-    <div
+    <label
       className={`flex items-center rounded transition-all duration-100 ease-in-out border ${
         focussed ? 'border-blue-400 shadow-input-blue-focus' : 'border-grey-200'
       } ${disabled ? 'border-grey-100 bg-grey-10' : 'bg-white'}`}
       data-testid="input-container"
     >
-      <div className="relative flex-auto">
-        <label
-          htmlFor={inputId}
+      <span className="block relative flex-auto">
+        <span
           className={`px-3 w-full cursor-text absolute text-sm origin-top-left transition-all duration-100 ease-in-out truncate font-body ${
             hasContent || focussed
               ? 'transform scale-80 mt-1 ml-1 -left-px'
@@ -85,11 +59,9 @@ export const TextInput = ({
           data-testid="input-label"
         >
           {label}
-        </label>
+        </span>
         <input
           className="pb-1 pt-5 px-3 w-full font-body text-sm rounded focus:outline-none disabled:bg-grey-10 placeholder-transparent focus:placeholder-grey-500 text-grey-600 disabled:text-grey-300 disabled:cursor-default"
-          id={inputId}
-          aria-describedby={readerText ? readerId : undefined}
           data-testid="input-field"
           onChange={textFieldChange}
           {...{
@@ -101,14 +73,9 @@ export const TextInput = ({
             type,
           }}
         />
-      </div>
-      {readerText && (
-        <span data-testid="input-srtext" className="sr-only" id={readerId}>
-          {readerText}
-        </span>
-      )}
+      </span>
       {children}
-    </div>
+    </label>
   );
 };
 

yarn.lock

@@ -16701,7 +16701,6 @@ fsevents@^1.2.7:
     "@types/node": ^12.12.35
     "@types/react": ^16.9.34
     "@types/react-dom": ^16.9.6
-    "@types/uuid": ^7.0.2
     "@types/webpack": 4.41.16
     babel-loader: ^8.0.0
     classnames: ^2.2.6
@@ -16724,7 +16723,6 @@ fsevents@^1.2.7:
     subscriptions-transport-ws: ^0.9.11
     tailwindcss: ^1.4.6
     typescript: 3.9.7
-    uuid: ^8.3.0
     webpack: ^4.43.0
   languageName: unknown
   linkType: soft