[MA-11]: Fix Screen reader speaking wrong item in list in Find Channe…

…ls modal
Brightscout · Dec 13, 2024 · f8ecb4a · f8ecb4a
1 parent 6e2c34b
commit f8ecb4a
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 36 deletions.
diff --git a/...ts/cypress/tests/integration/channels/archived_channel/archive_channel_operations_spec.ts b/...ts/cypress/tests/integration/channels/archived_channel/archive_channel_operations_spec.ts
@@ -57,7 +57,7 @@ describe('Leave an archived channel', () => {
 
         // * The archived channel appears in channel switcher search results
         cy.get('#suggestionList').should('be.visible');
-        cy.get('#suggestionList').find(`#switchChannel_${testChannel.name}`).should('be.visible');
+        cy.get('#suggestionList').find(`#switchChannel_${testChannel.id}`).should('be.visible');
 
         // # Reload the app (refresh the web page)
         cy.reload().then(() => {
@@ -68,7 +68,7 @@ describe('Leave an archived channel', () => {
             cy.get('#quickSwitchInput').type(testChannel.display_name).then(() => {
                 // * The archived channel appears in channel switcher search results
                 cy.get('#suggestionList').should('be.visible');
-                cy.get('#suggestionList').find(`#switchChannel_${testChannel.name}`).should('be.visible');
+                cy.get('#suggestionList').find(`#switchChannel_${testChannel.id}`).should('be.visible');
             });
         });
     });

diff --git a/...ts/cypress/tests/integration/channels/messaging/message_draft_then_switch_channel_spec.js b/...ts/cypress/tests/integration/channels/messaging/message_draft_then_switch_channel_spec.js
@@ -25,7 +25,7 @@ describe('Message Draft and Switch Channels', () => {
     });
 
     it('MM-T131 Message Draft Pencil Icon - CTRL/CMD+K & "Jump to"', () => {
-        const {name, display_name: displayName} = testChannel;
+        const {name, display_name: displayName, id} = testChannel;
         const message = 'message draft test';
 
         // * Validate if the draft icon is not visible at LHS before making a draft
@@ -55,10 +55,10 @@ describe('Message Draft and Switch Channels', () => {
         // * Suggestion list is visible
         cy.get('#suggestionList').should('be.visible').within(() => {
             // * A pencil icon before the channel name in the filtered list is visible
-            cy.get(`#switchChannel_${name}`).find('.icon-pencil-outline').should('be.visible');
+            cy.get(`#switchChannel_${id}`).find('.icon-pencil-outline').should('be.visible');
 
             // # Click to switch back to the test channel
-            cy.get(`#switchChannel_${name}`).click({force: true});
+            cy.get(`#switchChannel_${id}`).click({force: true});
         });
 
         // * Draft is saved in the text input box of the test channel

diff --git a/webapp/channels/src/components/search_bar/__snapshots__/search_bar.test.tsx.snap b/webapp/channels/src/components/search_bar/__snapshots__/search_bar.test.tsx.snap
@@ -32,13 +32,17 @@ exports[`components/search_bar/SearchBar should match snapshot with search 1`] =
           class="input-wrapper"
         >
           <input
+            aria-autocomplete="list"
             aria-describedby="searchbar-help-popup"
+            aria-expanded="false"
             aria-label="Search"
+            aria-owns="suggestionList"
             autocomplete="off"
             class="search-bar form-control a11y__region"
             data-a11y-sort-order="9"
             id="searchBox"
             placeholder="Search"
+            role="combobox"
             tabindex="0"
             type="search"
             value="test"
@@ -104,13 +108,17 @@ exports[`components/search_bar/SearchBar should match snapshot with search, with
           class="input-wrapper"
         >
           <input
+            aria-autocomplete="list"
             aria-describedby="searchbar-help-popup"
+            aria-expanded="false"
             aria-label="Search"
+            aria-owns="suggestionList"
             autocomplete="off"
             class="search-bar form-control a11y__region"
             data-a11y-sort-order="9"
             id="searchBox"
             placeholder="Search"
+            role="combobox"
             tabindex="0"
             type="search"
             value="test"
@@ -168,13 +176,17 @@ exports[`components/search_bar/SearchBar should match snapshot without search 1`
           class="input-wrapper"
         >
           <input
+            aria-autocomplete="list"
             aria-describedby="searchbar-help-popup"
+            aria-expanded="false"
             aria-label="Search"
+            aria-owns="suggestionList"
             autocomplete="off"
             class="search-bar form-control a11y__region"
             data-a11y-sort-order="9"
             id="searchBox"
             placeholder="Search"
+            role="combobox"
             tabindex="0"
             type="search"
             value=""
@@ -227,13 +239,17 @@ exports[`components/search_bar/SearchBar should match snapshot without search, w
           class="input-wrapper"
         >
           <input
+            aria-autocomplete="list"
             aria-describedby="searchbar-help-popup"
+            aria-expanded="false"
             aria-label="Search"
+            aria-owns="suggestionList"
             autocomplete="off"
             class="search-bar form-control a11y__region"
             data-a11y-sort-order="9"
             id="searchBox"
             placeholder="Search"
+            role="combobox"
             tabindex="0"
             type="search"
             value=""
@@ -278,13 +294,17 @@ exports[`components/search_bar/SearchBar should match snapshot without search, w
           class="input-wrapper"
         >
           <input
+            aria-autocomplete="list"
             aria-describedby="searchbar-help-popup"
+            aria-expanded="false"
             aria-label="Search"
+            aria-owns="suggestionList"
             autocomplete="off"
             class="search-bar form-control a11y__region"
             data-a11y-sort-order="9"
             id="searchBox"
             placeholder="Search"
+            role="combobox"
             tabindex="0"
             type="search"
             value=""

diff --git a/...mponents/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap b/...mponents/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap
@@ -34,7 +34,7 @@ exports[`at mention suggestion Should display nick name of non signed in user 1`
     onMouseMove={[MockFunction]}
     term="@user"
   >
-    <div
+    <li
       className="suggestion-list__item"
       data-testid="mentionSuggestion_user2"
       onClick={[Function]}
@@ -88,7 +88,7 @@ exports[`at mention suggestion Should display nick name of non signed in user 1`
           <div />
         </Component>
       </span>
-    </div>
+    </li>
   </SuggestionContainer>
 </AtMentionSuggestion>
 `;
@@ -129,7 +129,7 @@ exports[`at mention suggestion Should not display nick name of the signed in use
     onMouseMove={[MockFunction]}
     term="@user"
   >
-    <div
+    <li
       className="suggestion-list__item"
       data-testid="mentionSuggestion_user"
       onClick={[Function]}
@@ -191,7 +191,7 @@ exports[`at mention suggestion Should not display nick name of the signed in use
           <div />
         </Component>
       </span>
-    </div>
+    </li>
   </SuggestionContainer>
 </AtMentionSuggestion>
 `;
diff --git a/webapp/channels/src/components/suggestion/suggestion.tsx b/webapp/channels/src/components/suggestion/suggestion.tsx
@@ -4,7 +4,7 @@
 import classNames from 'classnames';
 import React, {useCallback} from 'react';
 
-export interface SuggestionProps<Item> extends Omit<React.HTMLAttributes<HTMLDivElement>, 'onClick' | 'onMouseMove'> {
+export interface SuggestionProps<Item> extends Omit<React.HTMLAttributes<HTMLLIElement>, 'onClick' | 'onMouseMove'> {
     // eslint-disable-next-line react/no-unused-prop-types
     item: Item;
 
@@ -17,7 +17,7 @@ export interface SuggestionProps<Item> extends Omit<React.HTMLAttributes<HTMLDiv
     onMouseMove: (term: string) => void;
 }
 
-const SuggestionContainer = React.forwardRef<HTMLDivElement, SuggestionProps<unknown>>((props, ref) => {
+const SuggestionContainer = React.forwardRef<HTMLLIElement, SuggestionProps<unknown>>((props, ref) => {
     const {
         children,
         term,
@@ -47,7 +47,7 @@ const SuggestionContainer = React.forwardRef<HTMLDivElement, SuggestionProps<unk
     }, [onMouseMove, term]);
 
     return (
-        <div
+        <li
             ref={ref}
             className={classNames('suggestion-list__item', {'suggestion--selected': isSelection})}
             onClick={handleClick}
@@ -57,7 +57,7 @@ const SuggestionContainer = React.forwardRef<HTMLDivElement, SuggestionProps<unk
             {...otherProps}
         >
             {children}
-        </div>
+        </li>
     );
 });
 

diff --git a/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx b/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx
@@ -820,6 +820,12 @@ export default class SuggestionBox extends React.PureComponent {
                     ref={this.inputRef}
                     autoComplete='off'
                     {...props}
+                    aria-owns='suggestionList'
+                    role='combobox'
+                    {...(this.state.selection && {'aria-activedescendant': `switchChannel_${this.state.selection}`}
+                    )}
+                    aria-autocomplete='list'
+                    aria-expanded={this.state.focused || this.props.forceSuggestionsWhenBlur}
                     onInput={this.handleChange}
                     onCompositionStart={this.handleCompositionStart}
                     onCompositionUpdate={this.handleCompositionUpdate}

diff --git a/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.test.tsx b/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.test.tsx
@@ -92,7 +92,7 @@ describe('SuggestionBox', () => {
         );
 
         // Start with no suggestions rendered
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
 
         // Typing some text should cause a suggestion to be shown
         userEvent.click(screen.getByPlaceholderText('test input'));
@@ -103,9 +103,9 @@ describe('SuggestionBox', () => {
             expect(providerSpy).toHaveBeenCalledTimes(1);
         });
 
-        expect(screen.queryByRole('list')).toBeVisible();
+        expect(screen.queryByRole('listbox')).toBeVisible();
 
-        expect(screen.queryByRole('list')).toBeVisible();
+        expect(screen.queryByRole('listbox')).toBeVisible();
         expect(screen.getByText('Suggestion: testtest')).toBeVisible();
 
         // Typing more text should cause the suggestion to be updaetd
@@ -115,13 +115,13 @@ describe('SuggestionBox', () => {
             expect(providerSpy).toHaveBeenCalledTimes(2);
         });
 
-        expect(screen.queryByRole('list')).toBeVisible();
+        expect(screen.queryByRole('listbox')).toBeVisible();
         expect(screen.getByText('Suggestion: testwordstestwords')).toBeVisible();
 
         // Clearing the textbox hides all suggestions
         await userEvent.clear(screen.getByPlaceholderText('test input'));
 
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
     });
 
     test('should hide suggestions on pressing escape', async () => {
@@ -135,20 +135,20 @@ describe('SuggestionBox', () => {
         );
 
         // Start with no suggestions rendered
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
 
         // Typing some text should cause a suggestion to be shown
         userEvent.click(screen.getByPlaceholderText('test input'));
         await userEvent.keyboard('test');
 
         await waitFor(() => {
-            expect(screen.getByRole('list')).toBeVisible();
+            expect(screen.getByRole('listbox')).toBeVisible();
         });
 
         // Pressing escape hides all suggestions
         await userEvent.keyboard('{escape}');
 
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
     });
 
     test('should autocomplete suggestions by pressing enter', async () => {
@@ -166,7 +166,7 @@ describe('SuggestionBox', () => {
         await userEvent.keyboard('test');
 
         await waitFor(() => {
-            expect(screen.queryByRole('list')).toBeVisible();
+            expect(screen.queryByRole('listbox')).toBeVisible();
             expect(screen.getByText('Suggestion: testtest')).toBeVisible();
         });
 
@@ -177,7 +177,7 @@ describe('SuggestionBox', () => {
             expect(screen.getByPlaceholderText('test input')).toHaveValue('testtest ');
         });
 
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
     });
 
     test('MM-57320 completing text with enter and calling resultCallback twice should not erase text following caret', async () => {
@@ -203,14 +203,14 @@ describe('SuggestionBox', () => {
         onSuggestionsReceived.mockClear();
 
         expect(screen.getByPlaceholderText('test input')).toHaveValue('This is important');
-        expect(screen.getByRole('list')).toBeVisible();
+        expect(screen.getByRole('listbox')).toBeVisible();
         expect(screen.getByText('Suggestion: This is importantThis is important')).toBeVisible();
 
         // Move the caret back to the start of the textbox and then use escape to clear the suggestions because
         // we don't support moving the caret with the autocomplete open yet
         await userEvent.keyboard('{home}{escape}');
 
-        expect(screen.queryByRole('list')).not.toBeInTheDocument();
+        expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
 
         // Type a space and then start typing something again to show results
         onSuggestionsReceived.mockClear();
@@ -221,7 +221,7 @@ describe('SuggestionBox', () => {
             expect(onSuggestionsReceived).toHaveBeenCalledTimes(2);
         });
 
-        expect(screen.getByRole('list')).toBeVisible();
+        expect(screen.getByRole('listbox')).toBeVisible();
         expect(screen.getByText('Suggestion: @us@us')).toBeVisible();
 
         onSuggestionsReceived.mockClear();

diff --git a/webapp/channels/src/components/suggestion/suggestion_list.tsx b/webapp/channels/src/components/suggestion/suggestion_list.tsx
@@ -44,7 +44,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
         renderDividers: [],
         renderNoResults: false,
     };
-    contentRef: React.RefObject<HTMLDivElement>;
+    contentRef: React.RefObject<HTMLUListElement>;
     wrapperRef: React.RefObject<HTMLDivElement>;
     itemRefs: Map<string, any>;
     currentLabel: string | null;
@@ -209,6 +209,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
             <div
                 key={type + '-divider'}
                 className='suggestion-list__divider'
+                role='separator'
             >
                 <span>
                     <FormattedMessage id={id}/>
@@ -219,7 +220,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
 
     renderNoResults() {
         return (
-            <div
+            <ul
                 key='list-no-results'
                 className='suggestion-list__no-results'
                 ref={this.contentRef}
@@ -232,7 +233,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
                         b: (chunks: string) => <b>{chunks}</b>,
                     }}
                 />
-            </div>
+            </ul>
         );
     }
 
@@ -277,6 +278,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
 
             items.push(
                 <Component
+                    data-option-index={i}
                     key={term}
                     ref={(ref: any) => this.itemRefs.set(term, ref)}
                     item={this.props.items[i]}
@@ -296,10 +298,10 @@ export default class SuggestionList extends React.PureComponent<Props> {
                 ref={this.wrapperRef}
                 className={mainClass}
             >
-                <div
+                <ul
                     id='suggestionList'
                     data-testid='suggestionList'
-                    role='list'
+                    role='listbox'
                     ref={this.contentRef}
                     style={{
                         maxHeight: this.maxHeight,
@@ -309,7 +311,7 @@ export default class SuggestionList extends React.PureComponent<Props> {
                     onMouseDown={this.props.preventClose}
                 >
                     {items}
-                </div>
+                </ul>
             </div>
         );
     }