diff --git a/webapp/channels/src/components/threading/global_threads/thread_item/__snapshots__/thread_item.test.tsx.snap b/webapp/channels/src/components/threading/global_threads/thread_item/__snapshots__/thread_item.test.tsx.snap index 004e7f41433..daa9db104d5 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_item/__snapshots__/thread_item.test.tsx.snap +++ b/webapp/channels/src/components/threading/global_threads/thread_item/__snapshots__/thread_item.test.tsx.snap @@ -51,26 +51,7 @@ exports[`components/threading/global_threads/thread_item should report total num isFollowing={true} threadId="1y8hpek81byspd4enyk9mp1ncw" unreadTimestamp={1611786714912} - > - - } - > - - - - - + />
- - } - > - - - - - + />
- - } - > - - - - - + />
- - )} - > - - - + />
{/* The strange interaction here where we need a click/keydown handler messes with the ESLint rules, so we just disable it */} diff --git a/webapp/channels/src/components/threading/global_threads/thread_menu/__snapshots__/thread_menu.test.tsx.snap b/webapp/channels/src/components/threading/global_threads/thread_menu/__snapshots__/thread_menu.test.tsx.snap deleted file mode 100644 index 90608337801..00000000000 --- a/webapp/channels/src/components/threading/global_threads/thread_menu/__snapshots__/thread_menu.test.tsx.snap +++ /dev/null @@ -1,87 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`components/threading/common/thread_menu should match snapshot 1`] = ` - - - - - - - - - - -`; - -exports[`components/threading/common/thread_menu should match snapshot after opening 1`] = ` - - - - - - - - - - -`; diff --git a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.scss b/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.scss deleted file mode 100644 index 2fa41412632..00000000000 --- a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.scss +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -.ThreadMenu { - .MenuItem__help-text { - margin: 0; - } -} diff --git a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.test.tsx b/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.test.tsx index 700e411914f..b2dcb9b1c82 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.test.tsx +++ b/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.test.tsx @@ -1,8 +1,6 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {shallow} from 'enzyme'; -import set from 'lodash/set'; import React from 'react'; import type {ComponentProps} from 'react'; @@ -14,13 +12,11 @@ import { } from 'actions/post_actions'; import {manuallyMarkThreadAsUnread} from 'actions/views/threads'; -import Menu from 'components/widgets/menu/menu'; - +import mergeObjects from 'packages/mattermost-redux/test/merge_objects'; import {fakeDate} from 'tests/helpers/date'; +import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils'; import {copyToClipboard} from 'utils/utils'; -import type {GlobalState} from 'types/store'; - import ThreadMenu from '../thread_menu'; jest.mock('mattermost-redux/actions/threads'); @@ -46,162 +42,245 @@ jest.mock('../../hooks', () => { }); const mockDispatch = jest.fn(); -let mockState: GlobalState; jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux') as typeof import('react-redux'), - useSelector: (selector: (state: typeof mockState) => unknown) => selector(mockState), useDispatch: () => mockDispatch, })); describe('components/threading/common/thread_menu', () => { let props: ComponentProps; + const baseState = { + entities: { + preferences: {myPreferences: {}}, + teams: {currentTeamId: 'tid'}, + general: {config: {}}, + users: {currentUserId: 'uid'}, + }, + views: { + browser: { + windowSize: 'desktopView', + }, + }, + }; + beforeEach(() => { props = { threadId: '1y8hpek81byspd4enyk9mp1ncw', unreadTimestamp: 1610486901110, hasUnreads: false, isFollowing: false, - children: ( - - ), }; - - mockState = {entities: {preferences: {myPreferences: {}}}} as GlobalState; }); - test('should match snapshot', () => { - const wrapper = shallow( + test('should render thread menu button', () => { + renderWithContext( , + baseState, ); - expect(wrapper).toMatchSnapshot(); + expect(screen.getByRole('button', {name: 'More Actions'})).toBeInTheDocument(); }); - test('should match snapshot after opening', () => { - const wrapper = shallow( + test('should open menu when button is clicked', async () => { + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - expect(wrapper).toMatchSnapshot(); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + expect(screen.getByRole('menuitem', {name: /Follow thread/})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: /Open in channel/})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: /Mark as unread/})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: /Save/})).toBeInTheDocument(); + expect(screen.getByRole('menuitem', {name: /Copy link/})).toBeInTheDocument(); }); - test('should allow following', () => { - const wrapper = shallow( + test('should allow following', async () => { + renderWithContext( , ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Follow thread'}).simulate('click'); - expect(setThreadFollow).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', true); - expect(mockDispatch).toHaveBeenCalledTimes(1); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const followButton = await screen.findByRole('menuitem', {name: /Follow thread/}); + await userEvent.click(followButton); + + await waitFor(() => { + expect(setThreadFollow).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', true); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); }); - test('should allow unfollowing', () => { - const wrapper = shallow( + test('should allow unfollowing', async () => { + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Unfollow thread'}).simulate('click'); - expect(setThreadFollow).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', false); - expect(mockDispatch).toHaveBeenCalledTimes(1); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const unfollowButton = screen.getByRole('menuitem', {name: /Unfollow thread/}); + await userEvent.click(unfollowButton); + + await waitFor(() => { + expect(setThreadFollow).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', false); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); }); - test('should allow opening in channel', () => { - const wrapper = shallow( + test('should allow opening in channel', async () => { + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Open in channel'}).simulate('click'); - expect(mockRouting.goToInChannel).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); - expect(mockDispatch).not.toHaveBeenCalled(); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const openInChannelButton = screen.getByRole('menuitem', {name: /Open in channel/}); + await userEvent.click(openInChannelButton); + + await waitFor(() => { + expect(mockRouting.goToInChannel).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); + expect(mockDispatch).not.toHaveBeenCalled(); + }); }); - test('should allow marking as read', () => { + test('should allow marking as read', async () => { const resetFakeDate = fakeDate(new Date(1612582579566)); - const wrapper = shallow( + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Mark as read'}).simulate('click'); - expect(markLastPostInThreadAsUnread).not.toHaveBeenCalled(); - expect(updateThreadRead).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', 1612582579566); - expect(manuallyMarkThreadAsUnread).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw', 1612582579566); - expect(mockDispatch).toHaveBeenCalledTimes(2); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const markAsReadButton = screen.getByRole('menuitem', {name: /Mark as read/}); + await userEvent.click(markAsReadButton); + + await waitFor(() => { + expect(markLastPostInThreadAsUnread).not.toHaveBeenCalled(); + expect(updateThreadRead).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw', 1612582579566); + expect(manuallyMarkThreadAsUnread).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw', 1612582579566); + expect(mockDispatch).toHaveBeenCalledTimes(2); + }); resetFakeDate(); }); - test('should allow marking as unread', () => { - const wrapper = shallow( + test('should allow marking as unread', async () => { + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Mark as unread'}).simulate('click'); - expect(updateThreadRead).not.toHaveBeenCalled(); - expect(markLastPostInThreadAsUnread).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw'); - expect(manuallyMarkThreadAsUnread).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw', 1610486901110); - expect(mockDispatch).toHaveBeenCalledTimes(2); - }); - test('should allow saving', () => { - const wrapper = shallow( - , - ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Save'}).simulate('click'); - expect(savePost).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); - expect(mockDispatch).toHaveBeenCalledTimes(1); - }); - test('should allow unsaving', () => { - set(mockState, 'entities.preferences.myPreferences', { - 'flagged_post--1y8hpek81byspd4enyk9mp1ncw': { - user_id: 'uid', - category: 'flagged_post', - name: '1y8hpek81byspd4enyk9mp1ncw', - value: 'true', - }, + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const markAsUnreadButton = screen.getByRole('menuitem', {name: /Mark as unread/}); + await userEvent.click(markAsUnreadButton); + + await waitFor(() => { + expect(updateThreadRead).not.toHaveBeenCalled(); + expect(markLastPostInThreadAsUnread).toHaveBeenCalledWith('uid', 'tid', '1y8hpek81byspd4enyk9mp1ncw'); + expect(manuallyMarkThreadAsUnread).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw', 1610486901110); + expect(mockDispatch).toHaveBeenCalledTimes(2); }); - - const wrapper = shallow( - , - ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Unsave'}).simulate('click'); - expect(unsavePost).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); - expect(mockDispatch).toHaveBeenCalledTimes(1); }); - test('should allow link copying', () => { - const wrapper = shallow( + test('should allow saving', async () => { + renderWithContext( , + baseState, ); - wrapper.find('button').simulate('click'); - wrapper.find(Menu.ItemAction).find({text: 'Copy link'}).simulate('click'); - expect(copyToClipboard).toHaveBeenCalledWith('http://localhost:8065/team-name-1/pl/1y8hpek81byspd4enyk9mp1ncw'); - expect(mockDispatch).not.toHaveBeenCalled(); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const saveButton = screen.getByRole('menuitem', {name: /Save/}); + await userEvent.click(saveButton); + + await waitFor(() => { + expect(savePost).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); + }); + test('should allow unsaving', async () => { + renderWithContext( + , + mergeObjects(baseState, { + entities: { + preferences: { + myPreferences: { + 'flagged_post--1y8hpek81byspd4enyk9mp1ncw': { + user_id: 'uid', + category: 'flagged_post', + name: '1y8hpek81byspd4enyk9mp1ncw', + value: 'true', + }, + }, + }, + }, + }), + ); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const unsaveButton = screen.getByRole('menuitem', {name: /Unsave/}); + await userEvent.click(unsaveButton); + + await waitFor(() => { + expect(unsavePost).toHaveBeenCalledWith('1y8hpek81byspd4enyk9mp1ncw'); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); + }); + + test('should allow link copying', async () => { + renderWithContext( + , + baseState, + ); + + const menuButton = screen.getByRole('button', {name: 'More Actions'}); + await userEvent.click(menuButton); + + const copyLinkButton = screen.getByRole('menuitem', {name: /Copy link/}); + await userEvent.click(copyLinkButton); + + await waitFor(() => { + expect(copyToClipboard).toHaveBeenCalledWith('http://localhost:8065/team-name-1/pl/1y8hpek81byspd4enyk9mp1ncw'); + expect(mockDispatch).not.toHaveBeenCalled(); + }); }); }); diff --git a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.tsx b/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.tsx index 7a5001f78ae..70d01dac50d 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.tsx +++ b/webapp/channels/src/components/threading/global_threads/thread_menu/thread_menu.tsx @@ -2,10 +2,10 @@ // See LICENSE.txt for license information. import React, {memo, useCallback} from 'react'; -import type {ReactNode} from 'react'; -import {useIntl} from 'react-intl'; +import {FormattedMessage, useIntl} from 'react-intl'; import {useDispatch, useSelector} from 'react-redux'; +import {DotsVerticalIcon} from '@mattermost/compass-icons/components'; import type {UserThread} from '@mattermost/types/threads'; import {setThreadFollow, updateThreadRead, markLastPostInThreadAsUnread} from 'mattermost-redux/actions/threads'; @@ -17,8 +17,7 @@ import { } from 'actions/post_actions'; import {manuallyMarkThreadAsUnread} from 'actions/views/threads'; -import Menu from 'components/widgets/menu/menu'; -import MenuWrapper from 'components/widgets/menu/menu_wrapper'; +import * as Menu from 'components/menu'; import {useReadout} from 'hooks/useReadout'; import {getSiteURL} from 'utils/url'; @@ -28,13 +27,10 @@ import type {GlobalState} from 'types/store'; import {useThreadRouting} from '../../hooks'; -import './thread_menu.scss'; - type Props = { threadId: UserThread['id']; isFollowing?: boolean; hasUnreads: boolean; - children: ReactNode; unreadTimestamp: number; }; @@ -43,7 +39,6 @@ function ThreadMenu({ isFollowing = false, unreadTimestamp, hasUnreads, - children, }: Props) { const {formatMessage} = useIntl(); const dispatch = useDispatch(); @@ -85,106 +80,131 @@ function ThreadMenu({ ]); return ( - + ), + }} + menuButtonTooltip={{ + text: formatMessage({ + id: 'threading.threadHeader.menu', + defaultMessage: 'More Actions', + }), + }} + menu={{ + id: `thread-menu-dropdown-${threadId}`, + }} > - {children} - - { - dispatch(setThreadFollow(currentUserId, currentTeamId, threadId, !isFollowing)); - readAloud(isFollowing ? formatMessage({ - id: 'threading.threadMenu.unfollowed', - defaultMessage: 'Unfollowed thread', - }) : formatMessage({ - id: 'threading.threadMenu.followed', - defaultMessage: 'Followed thread', - })); - }, [currentUserId, currentTeamId, threadId, isFollowing, setThreadFollow, readAloud, formatMessage])} - /> - { - goToInChannel(threadId); - readAloud(formatMessage({ - id: 'threading.threadMenu.openingChannel', - defaultMessage: 'Opening channel', - })); - }, [threadId, readAloud, formatMessage])} - /> - + + + + ) : ( + <> + + + ) + } + onClick={useCallback(() => { + dispatch(setThreadFollow(currentUserId, currentTeamId, threadId, !isFollowing)); + readAloud(isFollowing ? formatMessage({ + id: 'threading.threadMenu.unfollowed', + defaultMessage: 'Unfollowed thread', }) : formatMessage({ - id: 'threading.threadMenu.markUnread', - defaultMessage: 'Mark as unread', - })} - onClick={handleReadUnread} - /> - - + + } + onClick={useCallback(() => { + goToInChannel(threadId); + readAloud(formatMessage({ + id: 'threading.threadMenu.openingChannel', + defaultMessage: 'Opening channel', + })); + }, [threadId, readAloud, formatMessage])} + /> + + ) : ( + + )} + onClick={handleReadUnread} + /> + + ) : ( + + )} + onClick={useCallback(() => { + dispatch(isSaved ? unsavePost(threadId) : savePost(threadId)); + readAloud(isSaved ? formatMessage({ + id: 'threading.threadMenu.unsaved', + defaultMessage: 'Unsaved', }) : formatMessage({ - id: 'threading.threadMenu.save', - defaultMessage: 'Save', - })} - onClick={useCallback(() => { - dispatch(isSaved ? unsavePost(threadId) : savePost(threadId)); - readAloud(isSaved ? formatMessage({ - id: 'threading.threadMenu.unsaved', - defaultMessage: 'Unsaved', - }) : formatMessage({ - id: 'threading.threadMenu.saved', - defaultMessage: 'Saved', - })); - }, [threadId, isSaved])} - /> - { - copyToClipboard(`${getSiteURL()}/${team}/pl/${threadId}`); - readAloud(formatMessage({ - id: 'threading.threadMenu.linkCopied', - defaultMessage: 'Link copied', - })); - }, [team, threadId])} - /> - - + id: 'threading.threadMenu.saved', + defaultMessage: 'Saved', + })); + }, [threadId, isSaved])} + /> + + } + onClick={useCallback(() => { + copyToClipboard(`${getSiteURL()}/${team}/pl/${threadId}`); + readAloud(formatMessage({ + id: 'threading.threadMenu.linkCopied', + defaultMessage: 'Link copied', + })); + }, [team, threadId])} + /> + ); } diff --git a/webapp/channels/src/components/threading/global_threads/thread_pane/__snapshots__/thread_pane.test.tsx.snap b/webapp/channels/src/components/threading/global_threads/thread_pane/__snapshots__/thread_pane.test.tsx.snap index 67e7dc1cad1..9d7a4c4e00a 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_pane/__snapshots__/thread_pane.test.tsx.snap +++ b/webapp/channels/src/components/threading/global_threads/thread_pane/__snapshots__/thread_pane.test.tsx.snap @@ -45,19 +45,7 @@ exports[`components/threading/global_threads/thread_pane should match snapshot 1 isFollowing={true} threadId="1y8hpek81byspd4enyk9mp1ncw" unreadTimestamp={1611786714912} - > - - - - - - + /> } /> diff --git a/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.scss b/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.scss index f65989b5563..875e8d33e5c 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.scss +++ b/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.scss @@ -13,6 +13,7 @@ justify-content: space-between; padding: 12px 16px; border-bottom: var(--border-default); + gap: 4px; grid-area: header; --button-separator-height: 24px; @@ -55,14 +56,6 @@ font-weight: 400; } } - - .MenuWrapper { - margin-left: 4px; - - .dropdown-menu { - min-width: 250px; - } - } } .ThreadViewer { diff --git a/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.tsx b/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.tsx index 1c91a074218..8387a5157fb 100644 --- a/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.tsx +++ b/webapp/channels/src/components/threading/global_threads/thread_pane/thread_pane.tsx @@ -6,7 +6,6 @@ import type {ReactNode} from 'react'; import {useIntl} from 'react-intl'; import {useSelector, useDispatch} from 'react-redux'; -import {DotsVerticalIcon} from '@mattermost/compass-icons/components'; import type {UserThread} from '@mattermost/types/threads'; import {setThreadFollow} from 'mattermost-redux/actions/threads'; @@ -14,7 +13,6 @@ import {makeGetChannel} from 'mattermost-redux/selectors/entities/channels'; import {getPost, makeGetPostsForThread} from 'mattermost-redux/selectors/entities/posts'; import Header from 'components/widgets/header'; -import WithTooltip from 'components/with_tooltip'; import type {GlobalState} from 'types/store'; @@ -118,18 +116,7 @@ const ThreadPane = ({ isFollowing={isFollowing} hasUnreads={Boolean(thread.unread_replies || thread.unread_mentions)} unreadTimestamp={unreadTimestamp} - > - - - - + /> )} /> diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index ed396b25d6a..35a8ff477fd 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5762,7 +5762,6 @@ "threading.numReplies": "{totalReplies, plural, =0 {Reply} =1 {# reply} other {# replies}}", "threading.threadHeader.menu": "More Actions", "threading.threadItem.ariaLabel": "Thread by {author}", - "threading.threadItem.menu": "Actions", "threading.threadItem.timestamp": "Last reply ", "threading.threadList.markRead": "Mark all threads as read", "threading.threadList.tabsLabel": "Filter visible threads",