RSP-1396 [Accessibility] Asterisk has ariaLabel and role presentation

- Fix gap between icon or necessityLabel With one child, and isRequired on the LabelBase: - remove the alt text from the required icon or set aria-hidden={true} to the necessityLabel span element. - ensure that the child also receives isRequired={true} so that it will communicate required state by adding aria-required="true" to the input. Otherwise, include alt text on the required icon or don't hide the necessityLabel with aria-hidden. If the LabelBase does not have isRequired, but isRequired is true for the child component, set isRequired on the LabelBase, and also set necessityIndicator = 'icon' if necessityIndicator is undefined on the LabelBase.
adobe · Dec 9, 2019 · 75316a7 · 75316a7
1 parent 14d5f0c
commit 75316a7
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 29 deletions.
diff --git a/packages/@adobe/spectrum-css-temp/components/fieldlabel/index.css b/packages/@adobe/spectrum-css-temp/components/fieldlabel/index.css
@@ -15,6 +15,7 @@ governing permissions and limitations under the License.
 :root {
   --spectrum-tableform-border-spacing: 0 var(--spectrum-global-dimension-size-300);
   --spectrum-tableform-margin: calc(var(--spectrum-global-dimension-size-250) * -1) 0;
+  --spectrum-fieldlabel-necessitylabel-gap: var(--spectrum-global-dimension-size-40);
 }
 
 /* topdoc
@@ -43,8 +44,8 @@ governing permissions and limitations under the License.
   text-align: start;
 }
 
-.spectrum-FieldLabel-label {
-  margin-inline-end: 5px;
+.react-spectrum-FieldLabel-necessityLabel {
+  margin-inline-start: var(--spectrum-fieldlabel-necessitylabel-gap);
 }
 
 .spectrum-FieldLabel-requiredIcon {

diff --git a/packages/@react-aria/label/src/useLabel.ts b/packages/@react-aria/label/src/useLabel.ts
@@ -21,8 +21,8 @@ export function useLabel(labelProps: LabelProps & DOMProps = {}, labelledCompone
   let labelAriaProps: AllHTMLAttributes<HTMLElement> = {
     id: labelId,
     // htmlFor attribute can only reference a single id hence no concat, prioritize user set htmlFor attribute
-    // htmlFor is a React way to specify the "for" attribute since "for" is a reserved word in JS 
-    htmlFor: htmlFor || labelFor || labelledComponentId 
+    // htmlFor is a React way to specify the "for" attribute since "for" is a reserved word in JS
+    htmlFor: htmlFor || labelFor || labelledComponentId
   };
 
   if (ariaLabelledby) {
@@ -40,7 +40,7 @@ export function useLabel(labelProps: LabelProps & DOMProps = {}, labelledCompone
   }
 
   let labelledComponentAriaProps: AllHTMLAttributes<HTMLElement> = {
-    id: labelledComponentId, 
+    id: labelledComponentId,
     'aria-labelledby': ariaLabelledby
   };
 

diff --git a/packages/@react-spectrum/form/src/LabelBase.tsx b/packages/@react-spectrum/form/src/LabelBase.tsx
@@ -4,6 +4,7 @@ import {FieldLabelBase} from './types';
 import {filterDOMProps} from '@react-spectrum/utils';
 import intlMessages from '../intl/*.json';
 import {mergeProps} from '@react-aria/utils';
+import {Provider} from '@react-spectrum/provider';
 import React, {forwardRef} from 'react';
 import styles from '@adobe/spectrum-css-temp/components/fieldlabel/vars.css';
 import {useLabel} from '@react-aria/label';
@@ -51,15 +52,6 @@ function LabelBase(props: SpectrumLabelBaseProps, ref: DOMRef<HTMLLabelElement &
   let domRef = useDOMRef(ref);
   let {styleProps} = useStyleProps(otherProps);
 
-  let formatMessage = useMessageFormatter(intlMessages);
-  let necessityLabel = isRequired ? formatMessage('(required)') : formatMessage('(optional)');
-  let icon = (
-    <Asterisk
-      className={classNames(styles, 'spectrum-FieldLabel-requiredIcon')}
-      size="S"
-      alt={formatMessage('(required)')} />
-    );
-
   let wrapper;
   let childArray = React.Children.toArray(children);
   let labelledComponentProps;
@@ -69,6 +61,15 @@ function LabelBase(props: SpectrumLabelBaseProps, ref: DOMRef<HTMLLabelElement &
 
   let {labelAriaProps, labelledComponentAriaProps} = useLabel(props, labelledComponentProps);
 
+  let formatMessage = useMessageFormatter(intlMessages);
+  let necessityLabel = isRequired ? formatMessage('(required)') : formatMessage('(optional)');
+  let icon = (
+    <Asterisk
+      className={classNames(styles, 'spectrum-FieldLabel-requiredIcon')}
+      size="S"
+      alt={childArray.length === 1 ? undefined : formatMessage('(required)').replace(/[(（(](.+?)[)）)]/g, '$1')} />
+  );
+
   // Only apply the generated aria props to child if there is a label
   if (childArray.length === 1 && label) {
     childArray[0] = React.cloneElement(
@@ -89,11 +90,9 @@ function LabelBase(props: SpectrumLabelBaseProps, ref: DOMRef<HTMLLabelElement &
     <label
       {...labelAriaProps}
       className={fieldLabelClassName}>
-      <span className={classNames(styles, 'spectrum-FieldLabel-label')}>
-        {label}
-      </span>
+      {label}
       {necessityIndicator && ' '}
-      {necessityIndicator === 'label' && <span>{necessityLabel}</span>}
+      {necessityIndicator === 'label' && <span className={classNames(styles, 'react-spectrum-FieldLabel-necessityLabel')} aria-hidden={isRequired && childArray.length === 1}>{necessityLabel}</span>}
       {necessityIndicator === 'icon' && isRequired && icon}
     </label>
   ) : (
@@ -112,7 +111,9 @@ function LabelBase(props: SpectrumLabelBaseProps, ref: DOMRef<HTMLLabelElement &
     return (
       <div {...filterDOMProps(otherProps)} {...styleProps} ref={domRef}>
         {fieldLabel}
-        {wrapper || childArray}
+        <Provider isRequired={isRequired}>
+          {wrapper || childArray}
+        </Provider>
       </div>
     );
   }
@@ -122,7 +123,7 @@ function LabelBase(props: SpectrumLabelBaseProps, ref: DOMRef<HTMLLabelElement &
     {
       ref: domRef,
       ...mergeProps(
-        fieldLabel.props, 
+        fieldLabel.props,
         {...filterDOMProps(otherProps), ...styleProps}
       )
     }

diff --git a/packages/@react-spectrum/form/stories/FieldLabel.stories.js b/packages/@react-spectrum/form/stories/FieldLabel.stories.js
@@ -35,6 +35,7 @@ storiesOf('FieldLabel', module)
         {renderTextfield({label: 'Required label', isRequired: true, necessityIndicator: 'label'})}
         {renderTextfield({label: 'Optional label', isRequired: false, necessityIndicator: 'label'})}
         {renderTextfield({label: 'React', isRequired: true, necessityIndicator: 'icon'})}
+        {render({label: 'Checkbox Group', isRequired: true, necessityIndicator: 'icon'})}
         {renderTextfield({label: 'React', isRequired: false, necessityIndicator: 'icon'})}
       </div>
     )

diff --git a/packages/@react-spectrum/form/test/FieldLabel.test.js b/packages/@react-spectrum/form/test/FieldLabel.test.js
@@ -1,10 +1,18 @@
 import {cleanup, render, within} from '@testing-library/react';
 import * as createId from '@react/react-spectrum/utils/createId';
 import {FieldLabel} from '../';
+import {Provider} from '@react-spectrum/provider';
 import React from 'react';
+import scaleMedium from '@adobe/spectrum-css-temp/vars/spectrum-medium-unique.css';
+import themeLight from '@adobe/spectrum-css-temp/vars/spectrum-light-unique.css';
 import * as useId from '@react-aria/utils/src/useId';
 import V2FieldLabel from '@react/react-spectrum/FieldLabel';
 
+let theme = {
+  light: themeLight,
+  medium: scaleMedium
+};
+
 describe('FieldLabel', () => {
   let label = 'hi';
   let labelFor = 'blah';
@@ -14,10 +22,12 @@ describe('FieldLabel', () => {
 
   function renderFieldLabel(FieldLabelComponent, props, numChildren) {
     let component = (
-      <FieldLabelComponent data-testid={datatestid} {...props} >
-        {numChildren > 0 && <button data-testid="testbutton">test child</button>}
-        {numChildren > 1 && <button data-testid="testbutton2">test child2</button>}
-      </FieldLabelComponent>
+      <Provider theme={theme}>
+        <FieldLabelComponent data-testid={datatestid} {...props} >
+          {numChildren > 0 && <button data-testid="testbutton">test child</button>}
+          {numChildren > 1 && <button data-testid="testbutton2">test child2</button>}
+        </FieldLabelComponent>
+      </Provider>
     );
     return render(component);
   }
@@ -68,7 +78,13 @@ describe('FieldLabel', () => {
     `('$Name will show an asterix when required with icon indicator', ({Component, numChildren, props}) => {
       renderFieldLabel(Component, {label, labelFor, ...props}, numChildren);
       let fieldLabel = findLabel();
-      expect(within(fieldLabel).getByRole('img')).toBeTruthy();
+      let icon = within(fieldLabel).getByRole('img');
+      expect(icon).toBeTruthy();
+      if (numChildren === 1) {
+        expect(icon).not.toHaveAttribute('aria-label');
+      } else {
+        expect(icon).toHaveAttribute('aria-label');
+      }
     });
 
     // Forwarding ref is v3 specific
@@ -193,6 +209,26 @@ describe('FieldLabel', () => {
       expect(button).toHaveAttribute('aria-labelledby', 'first');
     });
 
+    it.each`
+      Name               | Component
+      ${'v3 FieldLabel'} | ${FieldLabel}
+    `('$Name should render with a wrapping div with autogenerated child id and for', ({Component}) => {
+      useIdMock.mockReturnValueOnce('first');
+      useIdMock.mockReturnValueOnce('second');
+      createIdMock.mockReturnValueOnce('first');
+      createIdMock.mockReturnValueOnce('second');
+
+      let tree = renderFieldLabel(Component, {label}, 1);
+      let fieldLabel = findLabel();
+      expect(fieldLabel).toBeTruthy();
+      expect(fieldLabel).toHaveAttribute('for', 'second');
+      expect(fieldLabel).toHaveAttribute('id', 'first');
+
+      let button = tree.getByRole('button');
+      expect(button).toHaveAttribute('id', 'second');
+      expect(button).toHaveAttribute('aria-labelledby', 'first');
+    });
+
     it.each`
       Name               | Component
       ${'v3 FieldLabel'} | ${FieldLabel}

diff --git a/packages/@react-spectrum/form/test/FormItem.test.js b/packages/@react-spectrum/form/test/FormItem.test.js
@@ -1,10 +1,18 @@
 import {cleanup, render, within} from '@testing-library/react';
 import * as createId from '@react/react-spectrum/utils/createId';
 import {FormItem} from '../';
+import {Provider} from '@react-spectrum/provider';
 import React from 'react';
+import scaleMedium from '@adobe/spectrum-css-temp/vars/spectrum-medium-unique.css';
+import themeLight from '@adobe/spectrum-css-temp/vars/spectrum-light-unique.css';
 import * as useId from '@react-aria/utils/src/useId';
 import {FormItem as V2FormItem} from '@react/react-spectrum/Form';
 
+let theme = {
+  light: themeLight,
+  medium: scaleMedium
+};
+
 describe('FormItem', () => {
   let label = 'hi';
   let labelFor = 'blah';
@@ -14,10 +22,12 @@ describe('FormItem', () => {
 
   function renderFormItem(FormItemComponent, props, numChildren) {
     let component = (
-      <FormItemComponent data-testid={datatestid} {...props} >
-        {numChildren > 0 && <button data-testid="testbutton">test child</button>}
-        {numChildren > 1 && <button data-testid="testbutton2">test child2</button>}
-      </FormItemComponent>
+      <Provider theme={theme}>
+        <FormItemComponent data-testid={datatestid} {...props} >
+          {numChildren > 0 && <button data-testid="testbutton">test child</button>}
+          {numChildren > 1 && <button data-testid="testbutton2">test child2</button>}
+        </FormItemComponent>
+      </Provider>
     );
     return render(component);
   }