From a03cc134bbd909d8edceb0619e2343f05102c06e Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Fri, 8 Aug 2025 12:27:06 -0400 Subject: [PATCH] MM-64868 Update accessible labels and descriptions for various autocompletes (#33553) * Remove the word 'icon' from labels for status icons * Remove unneeded alt text from at mention suggestion icons * MM-64868 Update label and description of AtMentionSuggestion * Update snapshots for other Suggestions * Remove redundant alt text from EmoticonSuggestion * Update label and description of ChannelMentionSuggestion I removed the readout of the channel slug because it's likely redundant, and I added the icon to it because whether or not the channel is public/private or archived seems useful to non-sighted users. * Remove icon from readout of GenericChannelSuggestion It doesn't depend on anything about the channel, so it's unnecessary. * Update label and description of GenericUserSuggestion Same as the AtMentionProvider, the profile image no longer has alt text, the mention is now the name, and the rest of the text is the description. * Update label and description of CommandSuggestion * Update label and description of SearchChannelWithPermissionsSuggestion Like ChannelMentionSuggestion, the name is the channel display name and the description is the type of channel. * Remove icon from readout of SearchUserSuggestion * Fix aria-label for ExtensionSuggestion not being read out * Update label and description of SearchChannelSuggestion Unlike the ChannelMentionSuggestion, I didn't include the channel type in this because, as of the previous PR, results are grouped by channel type. * Update label and description of SwitchChannelProvider This one has a lot because of all the different things in that modal, but I think everything relevant is read out. * fixup Update label and description of SwitchChannelProvider * Add some more unit tests --------- Co-authored-by: Mattermost Build --- .../builtin_commands/user_status_spec.js | 6 +- .../check_user_status_spec.js | 6 +- .../common/hooks/usePrefixedIds.test.ts | 88 ++++++ .../components/common/hooks/usePrefixedIds.ts | 29 ++ .../__snapshots__/panel_body.test.tsx.snap | 8 +- .../new_search/extension_suggestions.test.tsx | 1 + .../new_search/extension_suggestions.tsx | 3 +- .../post_profile_picture.test.tsx | 14 +- .../profile_popover/profile_popover.test.tsx | 4 +- .../components/shared_user_indicator.test.tsx | 2 +- .../src/components/shared_user_indicator.tsx | 2 +- .../channels/src/components/status_icon.tsx | 31 +- .../at_mention_suggestion.test.tsx.snap | 66 +++-- .../at_mention_suggestion.test.tsx | 80 +++++- .../at_mention_suggestion.tsx | 78 ++--- .../suggestion/channel_mention_provider.tsx | 60 +++- .../command_provider.test.tsx.snap | 5 + .../command_provider.test.tsx | 1 + .../command_provider/command_provider.tsx | 29 +- .../suggestion/emoticon_provider.tsx | 3 +- .../suggestion/generic_channel_provider.tsx | 5 +- .../suggestion/generic_user_provider.tsx | 24 +- .../search_channel_suggestion.test.tsx.snap | 43 ++- .../search_channel_suggestion.test.tsx | 1 + .../search_channel_suggestion.tsx | 29 +- ...arch_channel_with_permissions_provider.tsx | 52 +++- .../suggestion/search_user_provider.tsx | 1 + .../src/components/suggestion/suggestion.tsx | 4 +- .../switch_channel_provider.test.tsx | 268 +++++++++++++++++- .../suggestion/switch_channel_provider.tsx | 144 ++++++++-- .../user_group_popover.test.tsx.snap | 10 +- .../group_member_list.test.tsx.snap | 10 +- .../widgets/icons/status_away_icon.tsx | 2 +- .../widgets/icons/status_dnd_icon.tsx | 2 +- .../widgets/icons/status_offline_icon.tsx | 2 +- .../widgets/icons/status_online_icon.tsx | 2 +- webapp/channels/src/i18n/en.json | 16 +- .../dom-accessibility-api+0.5.16.patch | 40 +++ 38 files changed, 992 insertions(+), 179 deletions(-) create mode 100644 webapp/channels/src/components/common/hooks/usePrefixedIds.test.ts create mode 100644 webapp/channels/src/components/common/hooks/usePrefixedIds.ts create mode 100644 webapp/patches/dom-accessibility-api+0.5.16.patch diff --git a/e2e-tests/cypress/tests/integration/channels/integrations/builtin_commands/user_status_spec.js b/e2e-tests/cypress/tests/integration/channels/integrations/builtin_commands/user_status_spec.js index c5487cfdb60..e41f5b1203a 100644 --- a/e2e-tests/cypress/tests/integration/channels/integrations/builtin_commands/user_status_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/integrations/builtin_commands/user_status_spec.js @@ -13,9 +13,9 @@ import * as TIMEOUTS from '../../../../fixtures/timeouts'; describe('Integrations', () => { - const away = {name: 'away', ariaLabel: 'Away Icon', message: 'You are now away', className: 'icon-clock'}; - const offline = {name: 'offline', ariaLabel: 'Offline Icon', message: 'You are now offline', className: 'icon-circle-outline'}; - const online = {name: 'online', ariaLabel: 'Online Icon', message: 'You are now online', className: 'icon-check', profileClassName: 'icon-check-circle'}; + const away = {name: 'away', ariaLabel: 'Away', message: 'You are now away', className: 'icon-clock'}; + const offline = {name: 'offline', ariaLabel: 'Offline', message: 'You are now offline', className: 'icon-circle-outline'}; + const online = {name: 'online', ariaLabel: 'Online', message: 'You are now online', className: 'icon-check', profileClassName: 'icon-check-circle'}; before(() => { // # Login as test user and go to off-topic diff --git a/e2e-tests/cypress/tests/integration/channels/multi_team_and_dm/check_user_status_spec.js b/e2e-tests/cypress/tests/integration/channels/multi_team_and_dm/check_user_status_spec.js index 7e0940a5bcf..4bee3c6a7d5 100644 --- a/e2e-tests/cypress/tests/integration/channels/multi_team_and_dm/check_user_status_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/multi_team_and_dm/check_user_status_spec.js @@ -17,8 +17,8 @@ describe('Multi-Team + DMs', () => { let userB; let testChannelUrl; - const away = {name: 'away', ariaLabel: 'Away Icon', message: 'You are now away', className: 'icon-clock'}; - const online = {name: 'online', ariaLabel: 'Online Icon', message: 'You are now online', className: 'icon-check', profileClassName: 'icon-check-circle'}; + const away = {name: 'away', ariaLabel: 'Away', message: 'You are now away', className: 'icon-clock'}; + const online = {name: 'online', ariaLabel: 'Online', message: 'You are now online', className: 'icon-check', profileClassName: 'icon-check-circle'}; before(() => { cy.apiInitSetup().then(({team, user}) => { @@ -66,7 +66,7 @@ describe('Multi-Team + DMs', () => { should('be.visible'). children('div.Avatar-IGMzc'). children(). - find('svg').should('have.attr', 'aria-label', 'Away Icon'); + find('svg').should('have.attr', 'aria-label', 'Away'); }); }); diff --git a/webapp/channels/src/components/common/hooks/usePrefixedIds.test.ts b/webapp/channels/src/components/common/hooks/usePrefixedIds.test.ts new file mode 100644 index 00000000000..473d807dbd6 --- /dev/null +++ b/webapp/channels/src/components/common/hooks/usePrefixedIds.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {renderHook} from '@testing-library/react-hooks'; + +import usePrefixedIds, {joinIds} from './usePrefixedIds'; + +describe('usePrefixedIds', () => { + test('should combine prefixes with suffixes and return the result', () => { + const {result} = renderHook((props) => usePrefixedIds(props.prefix, props.suffixes), { + initialProps: { + prefix: 'p1', + suffixes: { + s1: null, + s2: null, + s3: null, + }, + }, + }); + + expect(result.current).toEqual({ + s1: 'p1-s1', + s2: 'p1-s2', + s3: 'p1-s3', + }); + }); + + test('should recalculate the result only when the prefix changes', () => { + const {rerender, result} = renderHook((props) => usePrefixedIds(props.prefix, props.suffixes), { + initialProps: { + prefix: 'p1', + suffixes: { + s1: null, + s2: null, + s3: null, + }, + }, + }); + + expect(result.all.length).toBe(1); + + // Re-rendering without changing the props should return the same result + rerender(); + + expect(result.all.length).toBe(2); + expect(result.all[0]).toBe(result.all[1]); + + // Re-rendering without changing the prefix should return the same result, even if getSuffixes isn't memoized + rerender({ + prefix: 'p1', + suffixes: { + s1: null, + s2: null, + s3: null, + }, + }); + + expect(result.all.length).toBe(3); + expect(result.all[0]).toBe(result.all[1]); + expect(result.all[0]).toBe(result.all[2]); + + // Changing the prefix should cause it to recalculate + rerender({ + prefix: 'p2', + suffixes: { + s1: null, + s2: null, + s3: null, + }, + }); + + expect(result.all.length).toBe(4); + expect(result.all[0]).toBe(result.all[1]); + expect(result.all[0]).toBe(result.all[2]); + expect(result.all[0]).not.toBe(result.all[3]); + expect(result.current).toEqual({ + s1: 'p2-s1', + s2: 'p2-s2', + s3: 'p2-s3', + }); + }); +}); + +describe('joinIds', () => { + test('should concatenate together provided IDs without any falsy ones', () => { + expect(joinIds('aa', 'bb', '', 'dd', '', '', 'gg')).toEqual('aa bb dd gg'); + }); +}); diff --git a/webapp/channels/src/components/common/hooks/usePrefixedIds.ts b/webapp/channels/src/components/common/hooks/usePrefixedIds.ts new file mode 100644 index 00000000000..2057931876d --- /dev/null +++ b/webapp/channels/src/components/common/hooks/usePrefixedIds.ts @@ -0,0 +1,29 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {useMemo} from 'react'; + +/** + * Combines the given prefix with a number of suffixes to generate IDs for the children of a component. + */ +export default function usePrefixedIds>(prefix: string, suffixes: S): {[K in keyof S]: string} { + // This hook assumes that suffixes never change, so use the original version unless prefix changes + // eslint-disable-next-line react-hooks/exhaustive-deps + const memoizedSuffixes = useMemo(() => suffixes, [prefix]); + + return useMemo(() => { + const childIds = { + ...memoizedSuffixes, + } as {[K in keyof S]: string}; + + for (const suffix of Object.keys(memoizedSuffixes)) { + childIds[suffix as keyof S] = `${prefix}-${suffix}`; + } + + return childIds; + }, [prefix, memoizedSuffixes]); +} + +export function joinIds(...ids: string[]): string { + return ids.filter(Boolean).join(' '); +} diff --git a/webapp/channels/src/components/drafts/panel/__snapshots__/panel_body.test.tsx.snap b/webapp/channels/src/components/drafts/panel/__snapshots__/panel_body.test.tsx.snap index 6361d83cc7a..e67cbc270e5 100644 --- a/webapp/channels/src/components/drafts/panel/__snapshots__/panel_body.test.tsx.snap +++ b/webapp/channels/src/components/drafts/panel/__snapshots__/panel_body.test.tsx.snap @@ -96,7 +96,7 @@ exports[`components/drafts/panel/panel_body should have called handleFormattedTe className="status " > { const baseProps = { + id: 'test-suggestion', item: {type: 'test-type', label: 'test-label', value: 'test-value'}, term: 'test', matchedPretext: 'test', diff --git a/webapp/channels/src/components/new_search/extension_suggestions.tsx b/webapp/channels/src/components/new_search/extension_suggestions.tsx index 72e5767256d..c3d239fd3a0 100644 --- a/webapp/channels/src/components/new_search/extension_suggestions.tsx +++ b/webapp/channels/src/components/new_search/extension_suggestions.tsx @@ -103,7 +103,7 @@ const messages: Record = const SearchFileExtensionSuggestion = React.forwardRef< HTMLDivElement, SuggestionProps ->(({item, onClick, matchedPretext, isSelection}, ref) => { +>(({id, item, onClick, matchedPretext, isSelection}, ref) => { const intl = useIntl(); const optionClicked = useCallback(() => { @@ -132,6 +132,7 @@ SuggestionProps return ( { , ); - expect(screen.queryByLabelText('Online Icon')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Online')).not.toBeInTheDocument(); - // no status is given, 'Offline Icon' should be in the dom as a fallback - expect(screen.getByLabelText('Offline Icon')).toBeInTheDocument(); + // no status is given, 'Offline' should be in the dom as a fallback + expect(screen.getByLabelText('Offline')).toBeInTheDocument(); }); test('status is specified, default props', () => { @@ -50,12 +50,12 @@ describe('components/PostProfilePicture', () => { , ); - // status is given, 'Away Icon' should be in the dom - expect(screen.getByLabelText('Away Icon')).toBeInTheDocument(); + // status is given, 'Away' should be in the dom + expect(screen.getByLabelText('Away')).toBeInTheDocument(); - expect(screen.queryByLabelText('Online Icon')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Online')).not.toBeInTheDocument(); - expect(screen.queryByLabelText('Offline Icon')).not.toBeInTheDocument(); + expect(screen.queryByLabelText('Offline')).not.toBeInTheDocument(); expect(screen.getAllByRole('img')).toHaveLength(2); }); diff --git a/webapp/channels/src/components/profile_popover/profile_popover.test.tsx b/webapp/channels/src/components/profile_popover/profile_popover.test.tsx index 1386cf6b74c..e36144ecfed 100644 --- a/webapp/channels/src/components/profile_popover/profile_popover.test.tsx +++ b/webapp/channels/src/components/profile_popover/profile_popover.test.tsx @@ -264,7 +264,7 @@ describe('components/ProfilePopover', () => { (Client4.getCallsChannelState as jest.Mock).mockImplementation(async () => ({enabled: true})); test('should correctly handle remote users based on connection status', async () => { - // Test 1: Verify shared user indicator is shown for any remote user + // Test 1: Verify shared user is shown for any remote user { const [props, initialState] = getBasePropsAndState(); initialState.entities!.users!.profiles!.user1!.remote_id = 'fakeuser'; @@ -274,7 +274,7 @@ describe('components/ProfilePopover', () => { }; const {unmount} = renderWithPluginReducers(, initialState); - expect(await screen.findByLabelText('shared user indicator')).toBeInTheDocument(); + expect(await screen.findByLabelText('shared user')).toBeInTheDocument(); unmount(); } diff --git a/webapp/channels/src/components/shared_user_indicator.test.tsx b/webapp/channels/src/components/shared_user_indicator.test.tsx index 38976d8c425..3a54c944857 100644 --- a/webapp/channels/src/components/shared_user_indicator.test.tsx +++ b/webapp/channels/src/components/shared_user_indicator.test.tsx @@ -21,7 +21,7 @@ describe('components/SharedUserIndicator', () => { return `From: ${descriptor.values?.remoteNames}`; } if (descriptor.id === 'shared_user_indicator.aria_label') { - return 'shared user indicator'; + return 'shared user'; } return descriptor.defaultMessage || ''; }), diff --git a/webapp/channels/src/components/shared_user_indicator.tsx b/webapp/channels/src/components/shared_user_indicator.tsx index a285c6d8800..62035da2384 100644 --- a/webapp/channels/src/components/shared_user_indicator.tsx +++ b/webapp/channels/src/components/shared_user_indicator.tsx @@ -30,7 +30,7 @@ const SharedUserIndicator = (props: Props) => { ); diff --git a/webapp/channels/src/components/status_icon.tsx b/webapp/channels/src/components/status_icon.tsx index 85fc33b8f8e..218171304a7 100644 --- a/webapp/channels/src/components/status_icon.tsx +++ b/webapp/channels/src/components/status_icon.tsx @@ -9,12 +9,14 @@ import StatusOfflineIcon from 'components/widgets/icons/status_offline_icon'; import StatusOnlineIcon from 'components/widgets/icons/status_online_icon'; type Props = { + id?: string; button?: boolean; status?: string; className?: string; } const StatusIcon = ({ + id, className = '', button = false, status, @@ -29,14 +31,27 @@ const StatusIcon = ({ iconClassName = className || ''; } - if (status === 'online') { - return ; - } else if (status === 'away') { - return ; - } else if (status === 'dnd') { - return ; - } - return ; + const Icon = getIcon(status); + + return ( + + ); }; +function getIcon(status?: string) { + switch (status) { + case 'online': + return StatusOnlineIcon; + case 'away': + return StatusAwayIcon; + case 'dnd': + return StatusDndIcon; + default: + return StatusOfflineIcon; + } +} + export default memo(StatusIcon); diff --git a/webapp/channels/src/components/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap b/webapp/channels/src/components/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap index ea374a13a5a..7ce1df0b06b 100644 --- a/webapp/channels/src/components/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap +++ b/webapp/channels/src/components/suggestion/at_mention_provider/__snapshots__/at_mention_suggestion.test.tsx.snap @@ -1,7 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`at mention suggestion Should display nick name of non signed in user 1`] = ` +exports[`at mention suggestion should display nick name of non signed in user 1`] = `
  • user2 profile image - + @user2 - - a b (c) + + + a b (c) + `; -exports[`at mention suggestion Should not display nick name of the signed in user 1`] = ` +exports[`at mention suggestion should not display nick name of the signed in user 1`] = `
  • user profile image - + @user - - a b - - - (you) + a b - + + + + + (you) + + + () =>
    ); -jest.spyOn(Utils, 'getFullName').mockReturnValue('a b'); describe('at mention suggestion', () => { const userid1 = { @@ -31,6 +31,7 @@ describe('at mention suggestion', () => { } as Item; const baseProps = { + id: 'test-suggestion-1', matchedPretext: '@', term: '@user', isSelection: false, @@ -38,7 +39,7 @@ describe('at mention suggestion', () => { onMouseMove: jest.fn(), }; - it('Should not display nick name of the signed in user', () => { + test('should not display nick name of the signed in user', () => { const wrapper = mountWithIntl( { expect(wrapper.find('.suggestion-list__ellipsis').text()).not.toContain('a b (c)'); }); - it('Should display nick name of non signed in user', () => { + test('should display nick name of non signed in user', () => { const wrapper = mountWithIntl( { expect(wrapper.find('.suggestion-list__ellipsis').text()).toContain('a b (c)'); }); + + describe('accessible text', () => { + const testCases = [ + { + name: 'at-mention suggestions should be labeled with the user\'s username and described with other names', + term: '@test-user', + item: {...TestHelper.getUserMock({username: 'test-user', first_name: 'First', last_name: 'Last', nickname: 'Nickname'})}, + expectedLabel: '@test-user', + expectedDescription: 'First Last (Nickname)', + }, + { + name: 'at-mention suggestions should include status in the description', + term: '@test-user', + item: {...TestHelper.getUserMock({username: 'test-user', first_name: 'First', last_name: 'Last'}), status: 'online'}, + expectedLabel: '@test-user', + expectedDescription: 'First Last Online', + }, + { + name: 'at-mention suggestions should include if the user is the current user', + term: '@test-user', + item: {...TestHelper.getUserMock({username: 'test-user', first_name: 'First', last_name: 'Last'}), isCurrentUser: true}, + expectedLabel: '@test-user', + expectedDescription: 'First Last (you)', + }, + { + name: 'at-mention suggestions should include if the user is a bot', + term: '@test-user', + item: {...TestHelper.getUserMock({username: 'test-user', first_name: '', last_name: '', nickname: 'Nickname', is_bot: true})}, + expectedLabel: '@test-user', + expectedDescription: '(Nickname) BOT', + }, + { + name: 'at-mention suggestions should include if the user is a remote user', + term: '@test-user:remote', + item: {...TestHelper.getUserMock({username: 'test-user:remote', first_name: '', last_name: '', remote_id: 'remote1'})}, + expectedLabel: '@test-user:remote', + expectedDescription: 'shared user', + }, + { + name: 'group suggestions should be labeled with the group slug and described with the group name', + term: '@test-group', + item: TestHelper.getGroupMock({name: 'test-group', display_name: 'Test Group', member_count: 5}), + expectedLabel: '@test-group', + expectedDescription: '- Test Group 5 members', + }, + { + name: 'special mention suggestions should be labeled with the at-mention and described properly', + term: '@channel', + item: {username: 'channel'}, + expectedLabel: '@channel', + expectedDescription: 'Notifies everyone in this channel', + }, + ]; + + for (const testCase of testCases) { + test(testCase.name, () => { + renderWithContext( + , + ); + + const suggestion = document.getElementById(baseProps.id); + expect(suggestion).toBe(screen.getByLabelText(testCase.expectedLabel)); + expect(suggestion).toHaveAccessibleName(testCase.expectedLabel); + expect(suggestion).toHaveAccessibleDescription(testCase.expectedDescription); + }); + } + }); }); diff --git a/webapp/channels/src/components/suggestion/at_mention_provider/at_mention_suggestion.tsx b/webapp/channels/src/components/suggestion/at_mention_provider/at_mention_suggestion.tsx index 43e5d4c7802..b73e61c7a88 100644 --- a/webapp/channels/src/components/suggestion/at_mention_provider/at_mention_suggestion.tsx +++ b/webapp/channels/src/components/suggestion/at_mention_provider/at_mention_suggestion.tsx @@ -3,12 +3,13 @@ import React from 'react'; import type {ReactNode} from 'react'; -import {FormattedMessage, useIntl} from 'react-intl'; +import {FormattedMessage} from 'react-intl'; import type {UserProfile} from '@mattermost/types/users'; import {isGuest} from 'mattermost-redux/utils/user_utils'; +import usePrefixedIds, {joinIds} from 'components/common/hooks/usePrefixedIds'; import CustomStatusEmoji from 'components/custom_status/custom_status_emoji'; import SharedUserIndicator from 'components/shared_user_indicator'; import StatusIcon from 'components/status_icon'; @@ -37,9 +38,18 @@ function isGroup(o: unknown): o is Group { } const AtMentionSuggestion = React.forwardRef>((props, ref) => { - const {item} = props; + const {id, item} = props; - const intl = useIntl(); + const ids = usePrefixedIds(id, { + atMention: null, + description: null, + youElement: null, + status: null, + botTag: null, + sharedIcon: null, + guestTag: null, + groupMembers: null, + }); let itemname: string; let description: ReactNode; @@ -58,10 +68,7 @@ const AtMentionSuggestion = React.forwardRef
    + ); } + const ids = usePrefixedIds(id, { + label: null, + description: null, + }); + return ( {icon}
    -
    +
    {item.Suggestion.substring(1) + ' ' + item.Hint}
    -
    +
    {item.Description}
    diff --git a/webapp/channels/src/components/suggestion/emoticon_provider.tsx b/webapp/channels/src/components/suggestion/emoticon_provider.tsx index 4693e043b42..0982613f4a3 100644 --- a/webapp/channels/src/components/suggestion/emoticon_provider.tsx +++ b/webapp/channels/src/components/suggestion/emoticon_provider.tsx @@ -39,10 +39,9 @@ const EmoticonSuggestion = React.forwardRef
    {text}
    diff --git a/webapp/channels/src/components/suggestion/generic_channel_provider.tsx b/webapp/channels/src/components/suggestion/generic_channel_provider.tsx index 831f4f7fc75..2cd653848e1 100644 --- a/webapp/channels/src/components/suggestion/generic_channel_provider.tsx +++ b/webapp/channels/src/components/suggestion/generic_channel_provider.tsx @@ -22,7 +22,10 @@ const GenericChannelSuggestion = React.forwardRef + ); diff --git a/webapp/channels/src/components/suggestion/generic_user_provider.tsx b/webapp/channels/src/components/suggestion/generic_user_provider.tsx index f311c3e54d2..02e334e8b33 100644 --- a/webapp/channels/src/components/suggestion/generic_user_provider.tsx +++ b/webapp/channels/src/components/suggestion/generic_user_provider.tsx @@ -6,6 +6,7 @@ import React from 'react'; import {Client4} from 'mattermost-redux/client'; import {isGuest} from 'mattermost-redux/utils/user_utils'; +import usePrefixedIds, {joinIds} from 'components/common/hooks/usePrefixedIds'; import BotTag from 'components/widgets/tag/bot_tag'; import GuestTag from 'components/widgets/tag/guest_tag'; import Avatar from 'components/widgets/users/avatar'; @@ -19,7 +20,14 @@ import {SuggestionContainer} from './suggestion'; import type {SuggestionProps} from './suggestion'; const GenericUserSuggestion = React.forwardRef>((props, ref) => { - const {item} = props; + const {id, item} = props; + + const ids = usePrefixedIds(id, { + name: null, + description: null, + botTag: null, + guestTag: null, + }); const username = item.username; let description = ''; @@ -36,20 +44,26 @@ const GenericUserSuggestion = React.forwardRef
    - + {'@' + username} - {description} + {description}
    - {item.is_bot && } - {isGuest(item.roles) && } + {item.is_bot && } + {isGuest(item.roles) && } ); }); diff --git a/webapp/channels/src/components/suggestion/search_channel_suggestion/__snapshots__/search_channel_suggestion.test.tsx.snap b/webapp/channels/src/components/suggestion/search_channel_suggestion/__snapshots__/search_channel_suggestion.test.tsx.snap index c1f2465f770..137cad5c587 100644 --- a/webapp/channels/src/components/suggestion/search_channel_suggestion/__snapshots__/search_channel_suggestion.test.tsx.snap +++ b/webapp/channels/src/components/suggestion/search_channel_suggestion/__snapshots__/search_channel_suggestion.test.tsx.snap @@ -2,7 +2,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot 1`] = ` name @@ -51,7 +53,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot 1 exports[`components/suggestion/search_channel_suggestion should match snapshot, channel type DM_CHANNEL 1`] = ` @@ -87,6 +91,7 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, > @name @@ -96,7 +101,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, exports[`components/suggestion/search_channel_suggestion should match snapshot, channel type GM_CHANNEL 1`] = ` @name @@ -146,7 +153,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, exports[`components/suggestion/search_channel_suggestion should match snapshot, channel type OPEN_CHANNEL 1`] = ` name @@ -195,7 +204,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, exports[`components/suggestion/search_channel_suggestion should match snapshot, channel type PRIVATE_CHANNEL 1`] = ` name @@ -244,7 +255,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, exports[`components/suggestion/search_channel_suggestion should match snapshot, isSelection is false 1`] = ` name @@ -293,7 +306,9 @@ exports[`components/suggestion/search_channel_suggestion should match snapshot, exports[`components/suggestion/search_channel_suggestion should match snapshot, isSelection is true 1`] = ` name diff --git a/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.test.tsx b/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.test.tsx index e3dc52da947..a3fafed765f 100644 --- a/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.test.tsx +++ b/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.test.tsx @@ -12,6 +12,7 @@ describe('components/suggestion/search_channel_suggestion', () => { const mockChannel = TestHelper.getChannelMock(); const baseProps = { + id: 'test-suggestion', item: mockChannel, isSelection: false, currentUserId: 'userid1', diff --git a/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.tsx b/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.tsx index bdcac04414a..38842228a05 100644 --- a/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.tsx +++ b/webapp/channels/src/components/suggestion/search_channel_suggestion/search_channel_suggestion.tsx @@ -7,6 +7,7 @@ import type {Channel} from '@mattermost/types/channels'; import {getUserIdFromChannelName} from 'mattermost-redux/utils/channel_utils'; +import usePrefixedIds from 'components/common/hooks/usePrefixedIds'; import BotTag from 'components/widgets/tag/bot_tag'; import Avatar from 'components/widgets/users/avatar'; @@ -20,6 +21,7 @@ function itemToName(item: Channel, currentUserId: string): {icon: React.ReactEle if (item.type === Constants.DM_CHANNEL) { const profilePicture = ( @@ -76,8 +78,18 @@ type Props = SuggestionProps & { teammateIsBot: boolean; } -const SearchChannelSuggestion = React.forwardRef((props, ref) => { - const {item, teammateIsBot, currentUserId} = props; +const SearchChannelSuggestion = React.forwardRef(({ + id, + item, + teammateIsBot, + currentUserId, + ...otherProps +}, ref) => { + const ids = usePrefixedIds(id, { + name: null, + channelType: null, + botTag: null, + }); const nameObject = itemToName(item, currentUserId); if (!nameObject) { @@ -91,16 +103,23 @@ const SearchChannelSuggestion = React.forwardRef((props, r return ( {icon}
    - + {name} {description}
    - {tag} + {tag && {tag}}     ); }); diff --git a/webapp/channels/src/components/suggestion/search_channel_with_permissions_provider.tsx b/webapp/channels/src/components/suggestion/search_channel_with_permissions_provider.tsx index e1c3768329b..61703ae50e5 100644 --- a/webapp/channels/src/components/suggestion/search_channel_with_permissions_provider.tsx +++ b/webapp/channels/src/components/suggestion/search_channel_with_permissions_provider.tsx @@ -2,6 +2,7 @@ // See LICENSE.txt for license information. import React from 'react'; +import {useIntl} from 'react-intl'; import type {Channel} from '@mattermost/types/channels'; @@ -20,6 +21,8 @@ import {sortChannelsByTypeAndDisplayName} from 'mattermost-redux/utils/channel_u import store from 'stores/redux_store'; +import usePrefixedIds from 'components/common/hooks/usePrefixedIds'; + import {Constants} from 'utils/constants'; import Provider from './provider'; @@ -37,7 +40,9 @@ interface WrappedChannel { type ChannelSearchFunction = (teamId: string, channelPrefix: string) => Promise const SearchChannelWithPermissionsSuggestion = React.forwardRef>((props, ref) => { - const {item} = props; + const {formatMessage} = useIntl(); + + const {id, item} = props; const channel = item.channel; const channelIsArchived = channel.delete_at && channel.delete_at !== 0; @@ -45,26 +50,61 @@ const SearchChannelWithPermissionsSuggestion = React.forwardRef + ); } else if (channel.type === Constants.OPEN_CHANNEL) { icon = ( - + ); } else if (channel.type === Constants.PRIVATE_CHANNEL) { icon = ( - + ); } + const ids = usePrefixedIds(id, { + name: null, + channelType: null, + }); + return ( - {icon} + + {icon} +
    - {displayName} + + {displayName} +
    ); diff --git a/webapp/channels/src/components/suggestion/search_user_provider.tsx b/webapp/channels/src/components/suggestion/search_user_provider.tsx index 358babab4ac..87d530f755b 100644 --- a/webapp/channels/src/components/suggestion/search_user_provider.tsx +++ b/webapp/channels/src/components/suggestion/search_user_provider.tsx @@ -50,6 +50,7 @@ export const SearchUserSuggestion = React.forwardRef
    diff --git a/webapp/channels/src/components/suggestion/suggestion.tsx b/webapp/channels/src/components/suggestion/suggestion.tsx index e0b6448dead..24b2f1c39d6 100644 --- a/webapp/channels/src/components/suggestion/suggestion.tsx +++ b/webapp/channels/src/components/suggestion/suggestion.tsx @@ -4,7 +4,9 @@ import classNames from 'classnames'; import React, {useCallback} from 'react'; -export interface SuggestionProps extends Omit, 'onClick' | 'onMouseMove' | 'role'> { +export interface SuggestionProps extends Omit, 'id' | 'onClick' | 'onMouseMove' | 'role'> { + id: string; + // eslint-disable-next-line react/no-unused-prop-types item: Item; diff --git a/webapp/channels/src/components/suggestion/switch_channel_provider.test.tsx b/webapp/channels/src/components/suggestion/switch_channel_provider.test.tsx index f21b8e628f3..540a9e4e5ac 100644 --- a/webapp/channels/src/components/suggestion/switch_channel_provider.test.tsx +++ b/webapp/channels/src/components/suggestion/switch_channel_provider.test.tsx @@ -1,15 +1,21 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import React from 'react'; + +import type {Channel} from '@mattermost/types/channels'; import {CollapsedThreads} from '@mattermost/types/config'; +import type {Team} from '@mattermost/types/teams'; import type {UserProfile} from '@mattermost/types/users'; -import {Preferences} from 'mattermost-redux/constants'; +import {General, Preferences} from 'mattermost-redux/constants'; +import {renderWithContext, screen} from 'tests/react_testing_utils'; import mockStore from 'tests/test_store'; +import {StoragePrefixes} from 'utils/constants'; import {TestHelper} from 'utils/test_helper'; -import SwitchChannelProvider from './switch_channel_provider'; +import SwitchChannelProvider, {ConnectedSwitchChannelSuggestion} from './switch_channel_provider'; const latestPost = TestHelper.getPostMock({ id: 'latest_post_id', @@ -990,3 +996,261 @@ describe('components/SwitchChannelProvider', () => { })); }); }); + +describe('SwitchChannelSuggestion', () => { + const baseProps = { + id: 'test-suggestion', + matchedPretext: '', + isSelection: false, + onClick: jest.fn(), + onMouseMove: jest.fn(), + }; + + const currentUserId = 'currentUser'; + + const team1 = TestHelper.getTeamMock({id: 'team1', display_name: 'Team One'}); + const team2 = TestHelper.getTeamMock({id: 'team2', display_name: 'Team Two'}); + + function getBaseState(teams: Team[], channels: Channel[]): any { + return { + entities: { + channels: { + channels: channels.reduce((channelsMap, channel) => ({...channelsMap, [channel.id]: channel}), {}), + myMembers: channels.reduce((membersMap, channel) => ({ + ...membersMap, + [channel.id]: TestHelper.getChannelMembershipMock({channel_id: channel.id, user_id: currentUserId}), + }), {}), + }, + teams: { + teams: teams.reduce((teamsMap, team) => ({...teamsMap, [team.id]: team}), {}), + myMembers: teams.reduce((membersMap, team) => ({ + ...membersMap, + [team.id]: TestHelper.getTeamMembershipMock({team_id: team.id, user_id: currentUserId}), + }), {}), + }, + }, + }; + } + + test('should show the team name for channels if the user is on multiple teams', () => { + const channel1 = TestHelper.getChannelMock({id: 'channel1', team_id: 'team1', name: 'channel_one', display_name: 'Channel One'}); + + const {replaceStoreState} = renderWithContext( + , + getBaseState([team1], [channel1]), + ); + + const suggestion = document.getElementById(baseProps.id); + + // When the user is on only a single team, the channel's URL name is displayed + expect(screen.getByText(`~${channel1.name}`)).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel1.name} Public channel`); + + replaceStoreState(getBaseState([team1, team2], [channel1])); + + // When the user is on multiple teams, we show the team's display name instead + expect(screen.getByText(team1.display_name)).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`${team1.display_name} Public channel`); + }); + + test('should show the type of channel', () => { + const channel1 = TestHelper.getChannelMock({id: 'channel1', team_id: 'team1', name: 'channel_one', display_name: 'Channel One', type: General.OPEN_CHANNEL}); + const channel2 = TestHelper.getChannelMock({id: 'channel2', team_id: 'team1', name: 'channel_two', display_name: 'Channel Two', type: General.PRIVATE_CHANNEL}); + + const {rerender} = renderWithContext( + , + getBaseState([team1], [channel1, channel2]), + ); + + const suggestion = document.getElementById(baseProps.id); + + expect(screen.getByLabelText('Public channel')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel1.name} Public channel`); + + rerender( + , + ); + + expect(screen.getByLabelText('Private channel')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel2.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel2.name} Private channel`); + }); + + test('should show if the channel has a draft instead of the channel type', () => { + const channel1 = TestHelper.getChannelMock({id: 'channel1', team_id: 'team1', name: 'channel_one', display_name: 'Channel One'}); + const channel2 = TestHelper.getChannelMock({id: 'channel2', team_id: 'team1', name: 'channel_two', display_name: 'Channel Two'}); + + const testState = getBaseState([team1], [channel1, channel2]); + testState.storage = { + storage: { + [`${StoragePrefixes.DRAFT}${channel2.id}`]: { + value: TestHelper.getPostDraftMock({message: 'post draft'}), + }, + }, + }; + + const {rerender} = renderWithContext( + , + testState, + ); + + const suggestion = document.getElementById(baseProps.id); + + expect(screen.queryByLabelText('Has draft')).not.toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel1.name} Public channel`); + + rerender( + , + ); + + expect(screen.queryByLabelText('Has draft')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel2.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel2.name} Has draft`); + }); + + test('should show if the channel is archived instead of the channel type', () => { + const channel1 = TestHelper.getChannelMock({id: 'channel1', team_id: 'team1', name: 'channel_one', display_name: 'Channel One'}); + const channel2 = TestHelper.getChannelMock({id: 'channel2', team_id: 'team1', name: 'channel_two', display_name: 'Channel Two', delete_at: 1}); + + const {rerender} = renderWithContext( + , + getBaseState([team1], [channel1, channel2]), + ); + + const suggestion = document.getElementById(baseProps.id); + + expect(screen.queryByLabelText('Archved channel')).not.toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel1.name} Public channel`); + + rerender( + , + ); + + expect(screen.queryByLabelText('Archived channel')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel2.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel2.name} Archived channel`); + }); + + test('should show if the channel has unread mentions', () => { + const channel1 = TestHelper.getChannelMock({id: 'channel1', team_id: 'team1', name: 'channel_one', display_name: 'Channel One'}); + const channel2 = TestHelper.getChannelMock({id: 'channel2', team_id: 'team1', name: 'channel_two', display_name: 'Channel Two'}); + const channel3 = TestHelper.getChannelMock({id: 'channel3', team_id: 'team1', name: 'channel_three', display_name: 'Channel Three'}); + + const testState = getBaseState([team1], [channel1, channel2, channel3]); + testState.entities.channels.myMembers[channel1.id].mention_count = 0; + testState.entities.channels.myMembers[channel2.id].mention_count = 1; + testState.entities.channels.myMembers[channel3.id].mention_count = 5; + + const {rerender} = renderWithContext( + , + testState, + ); + + const suggestion = document.getElementById(baseProps.id); + + expect(screen.queryByLabelText(/unread/, {exact: false})).not.toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel1.display_name); + expect(suggestion).toHaveAccessibleDescription(`~${channel1.name} Public channel`); + + rerender( + , + ); + + expect(screen.queryByLabelText('1 unread notification')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel2.display_name); + expect(suggestion).toHaveAccessibleDescription(`1 unread notification ~${channel2.name} Public channel`); + + rerender( + , + ); + + expect(screen.queryByLabelText('5 unread notifications')).toBeInTheDocument(); + expect(suggestion).toHaveAccessibleName(channel3.display_name); + expect(suggestion).toHaveAccessibleDescription(`5 unread notifications ~${channel3.name} Public channel`); + }); +}); diff --git a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx index d021b53d1e5..dfdcc08dee9 100644 --- a/webapp/channels/src/components/suggestion/switch_channel_provider.tsx +++ b/webapp/channels/src/components/suggestion/switch_channel_provider.tsx @@ -3,8 +3,7 @@ import classNames from 'classnames'; import React from 'react'; -import {defineMessage, injectIntl} from 'react-intl'; -import type {WrappedComponentProps} from 'react-intl'; +import {defineMessage, useIntl} from 'react-intl'; import {connect, useSelector} from 'react-redux'; import type {Channel, ChannelMembership} from '@mattermost/types/channels'; @@ -53,6 +52,7 @@ import {isGuest} from 'mattermost-redux/utils/user_utils'; import {getPostDraft} from 'selectors/rhs'; import globalStore from 'stores/redux_store'; +import usePrefixedIds, {joinIds} from 'components/common/hooks/usePrefixedIds'; import CustomStatusEmoji from 'components/custom_status/custom_status_emoji'; import ProfilePicture from 'components/profile_picture'; import SharedChannelIndicator from 'components/shared_channel_indicator'; @@ -114,7 +114,8 @@ export interface WrappedChannel { unread_mentions?: number; } -type Props = SuggestionProps & WrappedComponentProps & { +type Props = SuggestionProps & { + id: string; channelMember: ChannelMembership; collapsedThreads: boolean; dmChannelTeammate?: UserProfile; @@ -122,20 +123,38 @@ type Props = SuggestionProps & WrappedComponentProps & { isPartOfOnlyOneTeam: boolean; status?: string; team?: Team; - id: string; } -const SwitchChannelSuggestion = React.forwardRef((props, ref) => { - const {item, status, collapsedThreads, team, isPartOfOnlyOneTeam} = props; +export const SwitchChannelSuggestion = React.forwardRef(({ + id, + item, + channelMember: member, + collapsedThreads, + dmChannelTeammate: teammate, + hasDraft, + isPartOfOnlyOneTeam, + status, + team, + ...otherProps +}, ref) => { + const {formatMessage} = useIntl(); + const channel = item.channel; const channelIsArchived = channel.delete_at && channel.delete_at !== 0; const currentUserId = useSelector(getCurrentUserId); - const member = props.channelMember; - const teammate = props.dmChannelTeammate; - let badge = null; + const ids = usePrefixedIds(id, { + name: null, + channelType: null, + description: null, + sharedIcon: null, + tag: null, + teamName: null, + unreadBadge: null, + }); + let badge = null; if ((member && member.notify_props) || item.unread_mentions) { let unreadMentions; if (item.unread_mentions) { @@ -145,7 +164,16 @@ const SwitchChannelSuggestion = React.forwardRef((props, r } if (unreadMentions > 0 && !channelIsArchived) { badge = ( -
    +
    {unreadMentions} @@ -159,25 +187,53 @@ const SwitchChannelSuggestion = React.forwardRef((props, r let icon; if (channelIsArchived) { icon = ( - + ); - } else if (props.hasDraft) { + } else if (hasDraft) { icon = ( - + ); } else if (channel.type === Constants.OPEN_CHANNEL) { icon = ( - + ); } else if (channel.type === Constants.PRIVATE_CHANNEL) { icon = ( - + ); @@ -189,7 +245,14 @@ const SwitchChannelSuggestion = React.forwardRef((props, r ); } else if (channel.type === Constants.GM_CHANNEL) { icon = ( - +
    {'G'}
     ); @@ -224,7 +287,7 @@ const SwitchChannelSuggestion = React.forwardRef((props, r let deactivated = ''; if (teammate.delete_at) { - deactivated = (' - ' + props.intl.formatMessage({id: 'channel_switch_modal.deactivated', defaultMessage: 'Deactivated'})); + deactivated = (' - ' + formatMessage({id: 'channel_switch_modal.deactivated', defaultMessage: 'Deactivated'})); } if (channel.display_name && !(teammate && teammate.is_bot)) { @@ -232,7 +295,7 @@ const SwitchChannelSuggestion = React.forwardRef((props, r } else { name = teammate.username; if (teammate.id === currentUserId) { - name += (' ' + props.intl.formatMessage({id: 'suggestion.user.isCurrent', defaultMessage: '(you)'})); + name += (' ' + formatMessage({id: 'suggestion.user.isCurrent', defaultMessage: '(you)'})); } description = deactivated; } @@ -245,37 +308,60 @@ const SwitchChannelSuggestion = React.forwardRef((props, r let sharedIcon = null; if (isRealChannel(channel) && channel.shared) { sharedIcon = ( - + + + ); } let teamName = null; if (isRealChannel(channel) && channel.team_id && team) { - teamName = ({team.display_name}); + teamName = ( + + {team.display_name} + + ); } const showSlug = (isPartOfOnlyOneTeam || channel.type === Constants.DM_CHANNEL) && channel.type !== Constants.THREADS; + Reflect.deleteProperty(otherProps, 'dispatch'); + return ( {icon}
    {name} - {showSlug && description && {description}} + > + {name} + + {showSlug && description && ( + + {description} + + )} {customStatus} {sharedIcon} - {tag} + {tag && {tag}} {badge} {!isPartOfOnlyOneTeam && teamName}
    @@ -323,7 +409,7 @@ function mapStateToPropsForSwitchChannelSuggestion(state: GlobalState, ownProps: }; } -const ConnectedSwitchChannelSuggestion = connect(mapStateToPropsForSwitchChannelSuggestion, null, null, {forwardRef: true})(injectIntl(SwitchChannelSuggestion, {forwardRef: true})); +export const ConnectedSwitchChannelSuggestion = connect(mapStateToPropsForSwitchChannelSuggestion, null, null, {forwardRef: true})(SwitchChannelSuggestion); let prefix = ''; diff --git a/webapp/channels/src/components/user_group_popover/__snapshots__/user_group_popover.test.tsx.snap b/webapp/channels/src/components/user_group_popover/__snapshots__/user_group_popover.test.tsx.snap index 70b07b9369c..a529d675357 100644 --- a/webapp/channels/src/components/user_group_popover/__snapshots__/user_group_popover.test.tsx.snap +++ b/webapp/channels/src/components/user_group_popover/__snapshots__/user_group_popover.test.tsx.snap @@ -1045,7 +1045,7 @@ exports[`component/user_group_popover should match snapshot 1`] = ` className="status " > diff --git a/webapp/channels/src/components/widgets/icons/status_online_icon.tsx b/webapp/channels/src/components/widgets/icons/status_online_icon.tsx index db638e47a71..cd7a24e9a09 100644 --- a/webapp/channels/src/components/widgets/icons/status_online_icon.tsx +++ b/webapp/channels/src/components/widgets/icons/status_online_icon.tsx @@ -15,7 +15,7 @@ export default function StatusOnlineIcon(props: React.HTMLAttributes