[Terra-Form-Select] Fix for Form Single Select Voice Over Reader Issu…

…e. (#3855) Co-authored-by: st06365 <st063655@cerner.net> Co-authored-by: Saad Adnan <38024451+sdadn@users.noreply.github.com> Co-authored-by: Pravinkumar K S <pk106552@cerner.net> Co-authored-by: KV106606Viswanath <Kadali.Viswanath@cerner.com> Co-authored-by: ViswanathKadali <127083430+KV106606Viswanath@users.noreply.github.com> Co-authored-by: SM051274 <sm051274@cerner.net> Co-authored-by: Supreeth <supreeth.mr1990@gmail.com> Co-authored-by: saket2403 <saket.bajaj1998@gmail.com>
cerner · Oct 15, 2023 · b0fcee1 · b0fcee1
1 parent c5833c8
commit b0fcee1
Show file tree

Hide file tree

Showing 93 changed files with 163 additions and 121 deletions.
diff --git a/package.json b/package.json
@@ -119,7 +119,7 @@
     "stylelint": "^13.0.0",
     "terra-application": "^1.48.0",
     "terra-disclosure-manager": "^4.9.0",
-    "terra-enzyme-intl": "^3.0.0",
+    "terra-enzyme-intl": "^3.4.0",
     "webpack": "^5.28.0",
     "webpack-cli": "^4.6.0",
     "webpack-dev-server": "^4.11.1",

diff --git a/packages/terra-core-docs/CHANGELOG.md b/packages/terra-core-docs/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Added
+  * Added a visual error note for invalid example in `terra-form-single-select`.
+
 * Updated
   * Updated `terra-alert` documentation for custom titles.
 

diff --git a/packages/terra-core-docs/src/terra-dev-site/doc/form-select/example/single/Invalid.jsx b/packages/terra-core-docs/src/terra-dev-site/doc/form-select/example/single/Invalid.jsx
@@ -6,13 +6,16 @@ import styles from '../FormSelectDocCommon.module.scss';
 const cx = classNames.bind(styles);
 
 const InvalidExample = () => (
-  <SingleSelect placeholder="Select a color" isInvalid className={cx('form-select')}>
-    <SingleSelect.Option value="blue" display="Blue" />
-    <SingleSelect.Option value="green" display="Green" />
-    <SingleSelect.Option value="purple" display="Purple" />
-    <SingleSelect.Option value="red" display="Red" />
-    <SingleSelect.Option value="violet" display="Violet" />
-  </SingleSelect>
+  <>
+    <SingleSelect placeholder="Select a color" isInvalid className={cx('form-select')}>
+      <SingleSelect.Option value="blue" display="Blue" />
+      <SingleSelect.Option value="green" display="Green" />
+      <SingleSelect.Option value="purple" display="Purple" />
+      <SingleSelect.Option value="red" display="Red" />
+      <SingleSelect.Option value="violet" display="Violet" />
+    </SingleSelect>
+    <p>Note: User should always use form single select field inorder to convey the error state when error has occurred. </p>
+  </>
 );
 
 export default InvalidExample;
diff --git a/packages/terra-form-select/CHANGELOG.md b/packages/terra-form-select/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Changed
+  * Fixed Cyclic navigation and added aria-required to single select.
+  * Added relevant roles to fix a11y issues. 
+
 ## 6.47.0 - (September 21, 2023)
 
 * Changed

diff --git a/packages/terra-form-select/src/shared/_MenuUtil.js b/packages/terra-form-select/src/shared/_MenuUtil.js
@@ -195,7 +195,10 @@ class MenuUtil {
   static findNext(object, value) {
     const options = MenuUtil.flatten(object, true);
     const index = options.findIndex(({ props }) => MenuUtil.isEqual(props.value, value));
-    return index === -1 ? null : options[Math.min(index + 1, options.length - 1)].props.value;
+    if (options.length - 1 === index) {
+      return options[0].props.value;
+    }
+    return index === -1 ? null : options[index + 1].props.value;
   }
 
   /**
@@ -218,8 +221,11 @@ class MenuUtil {
    */
   static findPrevious(object, value) {
     const options = MenuUtil.flatten(object, true);
-    const index = options.findIndex(({ props }) => MenuUtil.isEqual(props.value, value));
-    return index === -1 ? null : options[Math.max(index - 1, 0)].props.value;
+    let index = options.findIndex(({ props }) => MenuUtil.isEqual(props.value, value));
+    if (index === 0) {
+      index = options.length;
+    }
+    return index === -1 ? null : options[index - 1].props.value;
   }
 
   /**

diff --git a/packages/terra-form-select/src/shared/_Option.jsx b/packages/terra-form-select/src/shared/_Option.jsx
@@ -112,7 +112,6 @@ const Option = ({
    */
   if (SharedUtil.isSafari() && !('ontouchstart' in window)) {
     role = 'radio';
-
     if (variant === 'tag' || variant === 'multiple') {
       role = 'checkbox';
     }

diff --git a/packages/terra-form-select/src/single/Frame.jsx b/packages/terra-form-select/src/single/Frame.jsx
@@ -142,7 +142,6 @@ class Frame extends React.Component {
     this.handleMouseDown = this.handleMouseDown.bind(this);
     this.handleToggleMouseDown = this.handleToggleMouseDown.bind(this);
     this.handleToggleButtonMouseDown = this.handleToggleButtonMouseDown.bind(this);
-    this.role = this.role.bind(this);
     this.visuallyHiddenComponent = React.createRef();
     this.setSelectMenuRef = this.setSelectMenuRef.bind(this);
   }
@@ -374,16 +373,15 @@ class Frame extends React.Component {
   ariaLabel() {
     const { ariaLabel, disabled, intl } = this.props;
 
-    const defaultAriaLabel = intl.formatMessage({ id: 'Terra.form.select.ariaLabel' });
     const dimmed = intl.formatMessage({ id: 'Terra.form.select.dimmed' });
 
     // VO on iOS doesn't do a good job of announcing disabled stated. Here we append the phrase that
     // VO associates with disabled form controls.
     if ('ontouchstart' in window && disabled) {
-      return ariaLabel === undefined ? `${defaultAriaLabel} ${dimmed}` : `${ariaLabel} ${dimmed}`;
+      return ariaLabel === undefined ? '' : `${ariaLabel} ${dimmed}`;
     }
 
-    return ariaLabel === undefined ? defaultAriaLabel : ariaLabel;
+    return ariaLabel === undefined ? '' : ariaLabel;
   }
 
   /**
@@ -393,24 +391,15 @@ class Frame extends React.Component {
     const selectedText = this.props.intl.formatMessage({ id: 'Terra.form.select.selected' });
     let label;
     if (this.props.display) {
-      label = `${this.props.display} ${selectedText}, ${this.ariaLabel()}`;
+      // Added below condition to prevent VO from announcing placeholder and selected item name twice in Safari.
+      label = SharedUtil.isSafari() ? `${selectedText}, ${this.ariaLabel()}` : `${this.props.display} ${selectedText}, ${this.ariaLabel()}`;
     } else if (this.props.placeholder) {
-      label = `${this.props.placeholder}, ${this.ariaLabel()}`;
+      label = SharedUtil.isSafari() ? this.ariaLabel() : `${this.props.placeholder}, ${this.ariaLabel()}`;
     }
 
     return label || this.ariaLabel();
   }
 
-  /**
-   * Determines compatible role attribute to apply to select based on active variant and disabled prop
-   */
-  role() {
-    const { disabled } = this.props;
-    const role = SharedUtil.isSafari() ? 'group' : 'button';
-
-    return disabled ? undefined : role;
-  }
-
   /**
    * Renders descriptive text related to the select component to be available for screen readers
    */
@@ -420,13 +409,13 @@ class Frame extends React.Component {
     const comboboxState = this.state.isOpen ? intl.formatMessage({ id: 'Terra.form.select.expanded' })
       : intl.formatMessage({ id: 'Terra.form.select.collapsed' });
     const listOfOptionsTxt = intl.formatMessage({ id: 'Terra.form.select.listOfTotalOptions' });
-    const defaultUsageGuidanceTxt = intl.formatMessage({ id: 'Terra.form.select.defaultUsageGuidance' });
+    const defaultUsageGuidanceTxt = this.props.intl.formatMessage({ id: 'Terra.form.select.defaultUsageGuidance' });
 
     if ('ontouchstart' in window) {
       return listOfOptionsTxt;
     }
 
-    return `${comboboxState} ${listOfOptionsTxt} ${defaultUsageGuidanceTxt}`;
+    return this.props.disabled ? '' : `${comboboxState} ${listOfOptionsTxt} ${defaultUsageGuidanceTxt}`;
   }
 
   renderToggleButton() {
@@ -485,7 +474,6 @@ class Frame extends React.Component {
       customProps.className,
     );
 
-    const labelId = `terra-select-screen-reader-label-${uniqueid()}`;
     const displayId = `terra-select-screen-reader-display-${uniqueid()}`;
     const placeholderId = `terra-select-screen-reader-placeholder-${uniqueid()}`;
     const descriptionId = `terra-select-screen-reader-description-${uniqueid()}`;
@@ -510,7 +498,7 @@ class Frame extends React.Component {
     return (
       <div
         {...customProps}
-        role={this.role()}
+        role="combobox"
         data-terra-select-combobox
         aria-controls={!disabled && this.state.isOpen ? selectMenuId : undefined}
         aria-disabled={!!disabled}
@@ -519,20 +507,26 @@ class Frame extends React.Component {
         aria-haspopup={!disabled ? 'true' : undefined}
         aria-describedby={ariaDescribedBy}
         aria-owns={this.state.isOpen ? selectMenuId : undefined}
+        aria-required={required}
         className={selectClasses}
         onBlur={this.handleBlur}
         onFocus={this.handleFocus}
         onKeyDown={this.handleKeyDown}
         onMouseDown={this.handleMouseDown}
         tabIndex={disabled ? '-1' : '0'} // eslint-disable-line jsx-a11y/no-noninteractive-tabindex
         ref={(select) => { this.select = select; }}
+        aria-invalid={isInvalid}
       >
+        {/* Added this label and input to avoid accessibility violations after changing the role from button to combobox */}
+        <label hidden>
+          <input type="text" />
+        </label>
         <div className={cx('visually-hidden-component')} hidden>
           {/* Hidden attribute used to prevent VoiceOver on desktop from announcing this content twice */}
-          <span id={labelId}>{this.ariaLabel()}</span>
           <span id={descriptionId}>{this.renderDescriptionText()}</span>
         </div>
-        <div className={cx('display')} aria-label={this.ariaLabel()}>
+        {/* Added aria label to avoid announcing empty group by voice over in safari browser */}
+        <div className={cx('display')} aria-label={this.ariaLabel()} aria-hidden={disabled}>
           {this.getDisplay(displayId, placeholderId)}
         </div>
         {this.renderToggleButton()}

diff --git a/packages/terra-form-select/src/single/Menu.jsx b/packages/terra-form-select/src/single/Menu.jsx
@@ -96,7 +96,9 @@ class Menu extends React.Component {
   constructor(props) {
     super(props);
 
-    this.state = {};
+    this.state = {
+      expandedStateText: this.props.intl.formatMessage({ id: 'Terra.form.select.expanded' }),
+    };
 
     this.searchString = '';
     this.clearSearch = this.clearSearch.bind(this);
@@ -394,7 +396,7 @@ class Menu extends React.Component {
           variant: 'default',
           totalOptions: object.length,
           index: object.indexOf(option) + 1,
-          expandedStateText: this.props.intl.formatMessage({ id: 'Terra.form.select.expanded' }),
+          expandedStateText: this.state.expandedStateText,
           ofText: this.props.intl.formatMessage({ id: 'Terra.form.select.of' }),
           onMouseDown: () => { this.downOption = option; },
           onMouseUp: event => this.handleOptionClick(event, option),

diff --git a/packages/terra-form-select/tests/jest/MenuUtil.test.jsx b/packages/terra-form-select/tests/jest/MenuUtil.test.jsx
@@ -302,7 +302,7 @@ describe('MenuUtil', () => {
         <OptGroup label="2">{options.slice(2, 5)}</OptGroup>,
       ];
 
-      expect(MenuUtil.findNext(group, '5')).toEqual('5');
+      expect(MenuUtil.findNext(group, '5')).toEqual('1');
     });
 
     it('should return null if there is no match', () => {
@@ -341,7 +341,7 @@ describe('MenuUtil', () => {
       expect(MenuUtil.findPrevious(group, '2')).toEqual('1');
     });
 
-    it('should return the same option if it is the last one in the list', () => {
+    it('should return the last option if it is the last one in the list', () => {
       const options = [
         <Option key="1" value="1" display="1" />,
         <Option key="2" value="2" display="2" />,
@@ -355,7 +355,7 @@ describe('MenuUtil', () => {
         <OptGroup label="2">{options.slice(2, 5)}</OptGroup>,
       ];
 
-      expect(MenuUtil.findPrevious(group, '1')).toEqual('1');
+      expect(MenuUtil.findPrevious(group, '1')).toEqual('5');
     });
 
     it('should return null if there is no match', () => {

diff --git a/packages/terra-form-select/tests/jest/Option.test.jsx b/packages/terra-form-select/tests/jest/Option.test.jsx
@@ -1,3 +1,4 @@
+/* eslint-disable import/no-extraneous-dependencies */
 import React from 'react';
 import ThemeContextProvider from 'terra-theme-context/lib/ThemeContextProvider';