From f8ecb4ae7a7eb457c77b6152ab2226482fbfa94c Mon Sep 17 00:00:00 2001 From: SaurabhSharma-884 Date: Fri, 6 Dec 2024 17:04:02 +0530 Subject: [PATCH] [MA-11]: Fix Screen reader speaking wrong item in list in Find Channels modal --- .../archive_channel_operations_spec.ts | 4 +-- .../message_draft_then_switch_channel_spec.js | 6 ++--- .../__snapshots__/search_bar.test.tsx.snap | 20 ++++++++++++++ .../at_mention_suggestion.test.tsx.snap | 8 +++--- .../src/components/suggestion/suggestion.tsx | 8 +++--- .../suggestion_box/suggestion_box.jsx | 6 +++++ .../suggestion_box/suggestion_box.test.tsx | 26 +++++++++---------- .../components/suggestion/suggestion_list.tsx | 14 +++++----- .../suggestion/switch_channel_provider.tsx | 12 ++++++--- 9 files changed, 68 insertions(+), 36 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/archived_channel/archive_channel_operations_spec.ts b/e2e-tests/cypress/tests/integration/channels/archived_channel/archive_channel_operations_spec.ts index 8b794e590b1..3dcf0580498 100644 --- a/e2e-tests/cypress/tests/integration/channels/archived_channel/archive_channel_operations_spec.ts +++ b/e2e-tests/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/e2e-tests/cypress/tests/integration/channels/messaging/message_draft_then_switch_channel_spec.js b/e2e-tests/cypress/tests/integration/channels/messaging/message_draft_then_switch_channel_spec.js index f44baa8aed2..d07103b034c 100644 --- a/e2e-tests/cypress/tests/integration/channels/messaging/message_draft_then_switch_channel_spec.js +++ b/e2e-tests/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 index 00dc93e2617..b4cd289a10e 100644 --- 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" > -
-
+ `; @@ -129,7 +129,7 @@ exports[`at mention suggestion Should not display nick name of the signed in use onMouseMove={[MockFunction]} term="@user" > -
-
+ `; diff --git a/webapp/channels/src/components/suggestion/suggestion.tsx b/webapp/channels/src/components/suggestion/suggestion.tsx index 5ed17d74c3e..31ccda92690 100644 --- 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 extends Omit, 'onClick' | 'onMouseMove'> { +export interface SuggestionProps extends Omit, 'onClick' | 'onMouseMove'> { // eslint-disable-next-line react/no-unused-prop-types item: Item; @@ -17,7 +17,7 @@ export interface SuggestionProps extends Omit void; } -const SuggestionContainer = React.forwardRef>((props, ref) => { +const SuggestionContainer = React.forwardRef>((props, ref) => { const { children, term, @@ -47,7 +47,7 @@ const SuggestionContainer = React.forwardRef {children} - + ); }); diff --git a/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx b/webapp/channels/src/components/suggestion/suggestion_box/suggestion_box.jsx index ff8f58ea95e..0b4f19d842b 100644 --- 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 index 94f281d511f..d519787ebc3 100644 --- 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 index b4b6cd4e7f3..84d1a9a5cd3 100644 --- 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 { renderDividers: [], renderNoResults: false, }; - contentRef: React.RefObject; + contentRef: React.RefObject; wrapperRef: React.RefObject; itemRefs: Map; currentLabel: string | null; @@ -209,6 +209,7 @@ export default class SuggestionList extends React.PureComponent {
@@ -219,7 +220,7 @@ export default class SuggestionList extends React.PureComponent { renderNoResults() { return ( -
{ b: (chunks: string) => {chunks}, }} /> -
+ ); } @@ -277,6 +278,7 @@ export default class SuggestionList extends React.PureComponent { items.push( this.itemRefs.set(term, ref)} item={this.props.items[i]} @@ -296,10 +298,10 @@ export default class SuggestionList extends React.PureComponent { ref={this.wrapperRef} className={mainClass} > -
{ onMouseDown={this.props.preventClose} > {items} -
+
); } diff --git a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx index d96b40d579f..7f7dd9cd5c7 100644 --- a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx +++ b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx @@ -121,7 +121,7 @@ type Props = SuggestionProps & { team?: Team; } -const SwitchChannelSuggestion = React.forwardRef((props, ref) => { +const SwitchChannelSuggestion = React.forwardRef((props, ref) => { const {item, status, collapsedThreads, team, isPartOfOnlyOneTeam} = props; const channel = item.channel; const channelIsArchived = channel.delete_at && channel.delete_at !== 0; @@ -256,15 +256,19 @@ const SwitchChannelSuggestion = React.forwardRef((props, return ( {icon}
- {name} + {name} {showSlug && description && {description}} {customStatus}