From 4890715b814a91f2763dd9f05b0a3c77fe347dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 11 Dec 2023 11:32:44 +0100 Subject: [PATCH] [MM-43331] No autofocus after RHS supression (#25593) * No autofocus after RHS supression * Add tests and some fixes * Address feedback * Fix test --- .../__snapshots__/rhs_thread.test.tsx.snap | 1 + .../components/rhs_thread/rhs_thread.test.tsx | 1 + .../src/components/rhs_thread/rhs_thread.tsx | 3 + .../src/components/sidebar_right/index.ts | 2 + .../sidebar_right/sidebar_right.test.tsx | 74 ++++++++++ .../sidebar_right/sidebar_right.tsx | 11 +- .../threading/thread_viewer/thread_viewer.tsx | 2 + .../virtualized_thread_viewer.test.tsx | 130 ++++++++++++------ .../virtualized_thread_viewer.tsx | 3 +- 9 files changed, 181 insertions(+), 46 deletions(-) create mode 100644 webapp/channels/src/components/sidebar_right/sidebar_right.test.tsx diff --git a/webapp/channels/src/components/rhs_thread/__snapshots__/rhs_thread.test.tsx.snap b/webapp/channels/src/components/rhs_thread/__snapshots__/rhs_thread.test.tsx.snap index 18dee9121b..07ce992c9e 100644 --- a/webapp/channels/src/components/rhs_thread/__snapshots__/rhs_thread.test.tsx.snap +++ b/webapp/channels/src/components/rhs_thread/__snapshots__/rhs_thread.test.tsx.snap @@ -30,6 +30,7 @@ exports[`components/RhsThread should match snapshot 1`] = ` rootPostId="id" /> { actions, directTeammate, currentTeam, + fromSuppressed: false, }; test('should match snapshot', () => { diff --git a/webapp/channels/src/components/rhs_thread/rhs_thread.tsx b/webapp/channels/src/components/rhs_thread/rhs_thread.tsx index cae8814007..614b999a47 100644 --- a/webapp/channels/src/components/rhs_thread/rhs_thread.tsx +++ b/webapp/channels/src/components/rhs_thread/rhs_thread.tsx @@ -21,6 +21,7 @@ type Props = { channel: Channel | null; selected: Post | FakePost; previousRhsState?: RhsState; + fromSuppressed: boolean; } const RhsThread = ({ @@ -29,6 +30,7 @@ const RhsThread = ({ posts, selected, previousRhsState, + fromSuppressed, }: Props) => { const dispatch = useDispatch(); @@ -59,6 +61,7 @@ const RhsThread = ({ rootPostId={selected.id} useRelativeTimestamp={false} isThreadView={false} + fromSuppressed={fromSuppressed} /> ); diff --git a/webapp/channels/src/components/sidebar_right/index.ts b/webapp/channels/src/components/sidebar_right/index.ts index 5d7723d15f..c8bebbbcbe 100644 --- a/webapp/channels/src/components/sidebar_right/index.ts +++ b/webapp/channels/src/components/sidebar_right/index.ts @@ -20,6 +20,7 @@ import { getSelectedPostId, getSelectedPostCardId, getPreviousRhsState, + getIsRhsSuppressed, } from 'selectors/rhs'; import {RHSStates} from 'utils/constants'; @@ -41,6 +42,7 @@ function mapStateToProps(state: GlobalState, props: RouteComponentProps) { return { isExpanded: getIsRhsExpanded(state), isOpen: getIsRhsOpen(state), + isSuppressed: getIsRhsSuppressed(state), channel, postRightVisible: Boolean(selectedPostId) && rhsState !== RHSStates.EDIT_HISTORY, postCardVisible: Boolean(selectedPostCardId), diff --git a/webapp/channels/src/components/sidebar_right/sidebar_right.test.tsx b/webapp/channels/src/components/sidebar_right/sidebar_right.test.tsx new file mode 100644 index 0000000000..7289d58bd9 --- /dev/null +++ b/webapp/channels/src/components/sidebar_right/sidebar_right.test.tsx @@ -0,0 +1,74 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {shallow} from 'enzyme'; +import type {ComponentProps} from 'react'; +import React from 'react'; + +import RhsThread from 'components/rhs_thread'; + +import {TestHelper} from 'utils/test_helper'; + +import SidebarRight from './sidebar_right'; + +type Props = ComponentProps; +function getBaseProps(): Props { + const channel = TestHelper.getChannelMock(); + return { + actions: { + closeRightHandSide: jest.fn(), + openAtPrevious: jest.fn(), + openRHSSearch: jest.fn(), + setRhsExpanded: jest.fn(), + showChannelFiles: jest.fn(), + showChannelInfo: jest.fn(), + showPinnedPosts: jest.fn(), + updateSearchTerms: jest.fn(), + }, + channel, + isChannelFiles: false, + isChannelInfo: false, + isChannelMembers: false, + isExpanded: false, + isOpen: false, + isPinnedPosts: false, + isPluginView: false, + isPostEditHistory: false, + isSuppressed: false, + postCardVisible: false, + postRightVisible: false, + previousRhsState: '', + productId: '', + rhsChannel: channel, + searchVisible: false, + selectedPostCardId: '', + selectedPostId: '', + team: TestHelper.getTeamMock(), + teamId: '', + }; +} +describe('pass from suppressed', () => { + it('fromSuppressed is only passed when moving from suppressed state to non suppressed', () => { + const props = getBaseProps(); + const wrapper = shallow(); + expect(wrapper.find(RhsThread)).toHaveLength(0); + + wrapper.setProps({isOpen: true, postRightVisible: true}); + expect(wrapper.find(RhsThread)).toHaveLength(1); + expect(wrapper.find(RhsThread).props().fromSuppressed).toBeFalsy(); + + wrapper.setProps({isSuppressed: true, isOpen: false}); + expect(wrapper.find(RhsThread)).toHaveLength(0); + + wrapper.setProps({isSuppressed: false, isOpen: true}); + expect(wrapper.find(RhsThread)).toHaveLength(1); + expect(wrapper.find(RhsThread).props().fromSuppressed).toBeTruthy(); + + wrapper.setProps({isOpen: false, postRightVisible: false}); + expect(wrapper.find(RhsThread)).toHaveLength(0); + + wrapper.setProps({isOpen: true, postRightVisible: true}); + expect(wrapper.find(RhsThread)).toHaveLength(1); + expect(wrapper.find(RhsThread).props().fromSuppressed).toBeFalsy(); + }); +}); diff --git a/webapp/channels/src/components/sidebar_right/sidebar_right.tsx b/webapp/channels/src/components/sidebar_right/sidebar_right.tsx index ee93d7d910..058e4f18af 100644 --- a/webapp/channels/src/components/sidebar_right/sidebar_right.tsx +++ b/webapp/channels/src/components/sidebar_right/sidebar_right.tsx @@ -28,6 +28,7 @@ import {isMac} from 'utils/user_agent'; import type {RhsState} from 'types/store/rhs'; export type Props = { + isSuppressed: boolean; isExpanded: boolean; isOpen: boolean; channel: Channel; @@ -68,6 +69,8 @@ export default class SidebarRight extends React.PureComponent { sidebarRightWidthHolder: React.RefObject; previous: Partial | undefined = undefined; focusSearchBar?: () => void; + lastOpenState = false; + lastSuppressedState = false; constructor(props: Props) { super(props); @@ -149,6 +152,9 @@ export default class SidebarRight extends React.PureComponent { trackEvent('ui', 'ui_rhs_opened'); } + this.lastOpenState = this.props.isOpen; + this.lastSuppressedState = this.props.isSuppressed; + const {actions, isChannelFiles, isPinnedPosts, rhsChannel, channel} = this.props; if (isPinnedPosts && prevProps.isPinnedPosts === isPinnedPosts && rhsChannel.id !== prevProps.rhsChannel.id) { actions.showPinnedPosts(rhsChannel.id); @@ -232,7 +238,10 @@ export default class SidebarRight extends React.PureComponent { content = (
- +
); } else if (postCardVisible) { diff --git a/webapp/channels/src/components/threading/thread_viewer/thread_viewer.tsx b/webapp/channels/src/components/threading/thread_viewer/thread_viewer.tsx index 08fcb1821d..c5dccb8a5e 100644 --- a/webapp/channels/src/components/threading/thread_viewer/thread_viewer.tsx +++ b/webapp/channels/src/components/threading/thread_viewer/thread_viewer.tsx @@ -50,6 +50,7 @@ export type Props = Attrs & { isThreadView?: boolean; inputPlaceholder?: string; rootPostId: string; + fromSuppressed?: boolean; }; type State = { @@ -224,6 +225,7 @@ export default class ThreadViewer extends React.PureComponent { highlightedPostId={this.props.highlightedPostId} selectedPostFocusedAt={this.props.selectedPostFocusedAt} isThreadView={Boolean(this.props.isCollapsedThreadsEnabled && this.props.isThreadView)} + fromSuppressed={this.props.fromSuppressed} /> )} diff --git a/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.test.tsx b/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.test.tsx index e6cde6b951..b8e11ed76b 100644 --- a/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.test.tsx +++ b/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.test.tsx @@ -1,71 +1,84 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import {screen} from '@testing-library/react'; import {shallow} from 'enzyme'; +import type {ComponentProps} from 'react'; import React from 'react'; -import type {Channel} from '@mattermost/types/channels'; -import type {Post} from '@mattermost/types/posts'; +import type {GlobalState} from '@mattermost/types/store'; import type {UserProfile} from '@mattermost/types/users'; +import type {DeepPartial} from '@mattermost/types/utilities'; +import {Permissions} from 'mattermost-redux/constants'; + +import {renderWithContext} from 'tests/react_testing_utils'; import {TestHelper} from 'utils/test_helper'; import VirtualizedThreadViewer from './virtualized_thread_viewer'; -describe('components/threading/VirtualizedThreadViewer', () => { - const post: Post = TestHelper.getPostMock({ - channel_id: 'channel_id', - create_at: 1502715365009, - update_at: 1502715372443, - is_following: true, - reply_count: 3, - }); +// Needed for apply markdown to properly work down the line +global.ResizeObserver = require('resize-observer-polyfill'); - const channel: Channel = TestHelper.getChannelMock({ - display_name: '', - name: '', - header: '', - purpose: '', - creator_id: '', - scheme_id: '', - teammate_id: '', - status: '', +type Props = ComponentProps; +function getBasePropsAndState(): [Props, DeepPartial] { + const channel = TestHelper.getChannelMock(); + const currentUser = TestHelper.getUserMock({roles: 'role'}); + const post = TestHelper.getPostMock({ + channel_id: channel.id, }); - const actions = { - removePost: jest.fn(), - selectPostCard: jest.fn(), - getPostThread: jest.fn(), - getThread: jest.fn(), - updateThreadRead: jest.fn(), - updateThreadLastOpened: jest.fn(), - fetchRHSAppsBindings: jest.fn(), - }; - const directTeammate: UserProfile = TestHelper.getUserMock(); - - const baseProps = { + const props: Props = { selected: post, channel, currentUserId: 'user_id', - currentTeamId: 'team_id', - previewCollapsed: 'false', - previewEnabled: true, - socketConnectionStatus: true, - actions, directTeammate, - posts: [post], lastPost: post, onCardClick: () => {}, - onCardClickPost: () => {}, - replyListIds: [], - teamId: '', + replyListIds: ['create-comment'], useRelativeTimestamp: true, isMobileView: false, - isThreadView: true, + isThreadView: false, lastViewedAt: 0, newMessagesSeparatorActions: [], + fromSuppressed: false, }; + + const state: DeepPartial = { + entities: { + users: { + currentUserId: currentUser.id, + profiles: { + [currentUser.id]: currentUser, + }, + }, + posts: { + posts: { + [post.id]: post, + }, + }, + channels: { + channels: { + [channel.id]: channel, + }, + }, + roles: { + roles: { + role: { + id: 'role', + name: 'role', + permissions: [Permissions.CREATE_POST, Permissions.USE_CHANNEL_MENTIONS], + }, + }, + }, + }, + }; + return [props, state]; +} + +describe('components/threading/VirtualizedThreadViewer', () => { + const [baseProps] = getBasePropsAndState(); test('should scroll to the bottom when the current user makes a new post in the thread', () => { const scrollToBottom = jest.fn(); @@ -80,7 +93,7 @@ describe('components/threading/VirtualizedThreadViewer', () => { lastPost: { id: 'newpost', - root_id: post.id, + root_id: baseProps.selected.id, user_id: 'user_id', }, }); @@ -103,7 +116,7 @@ describe('components/threading/VirtualizedThreadViewer', () => { lastPost: { id: 'newpost', - root_id: post.id, + root_id: baseProps.selected.id, user_id: 'other_user_id', }, }); @@ -127,7 +140,7 @@ describe('components/threading/VirtualizedThreadViewer', () => { lastPost: { id: 'newpost', - root_id: post.id, + root_id: baseProps.selected.id, user_id: 'user_id', }, highlightedPostId: '42', @@ -136,3 +149,32 @@ describe('components/threading/VirtualizedThreadViewer', () => { expect(scrollToBottom).not.toHaveBeenCalled(); }); }); + +describe('fromSuppressed works as expected', () => { + // This setup is so AutoSizer renders its contents + const originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight'); + const originalOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetWidth'); + + beforeAll(() => { + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {configurable: true, value: 50}); + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', {configurable: true, value: 50}); + }); + + afterAll(() => { + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', originalOffsetHeight!); + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', originalOffsetWidth!); + }); + + it('autofocus if fromSuppressed is not set', () => { + const [props, state] = getBasePropsAndState(); + renderWithContext(, state); + expect(screen.getByRole('textbox')).toHaveFocus(); + }); + + it('do not autofocus if fromSuppressed is set', () => { + const [props, state] = getBasePropsAndState(); + props.fromSuppressed = true; + renderWithContext(, state); + expect(screen.getByRole('textbox')).not.toHaveFocus(); + }); +}); diff --git a/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.tsx b/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.tsx index 0297b290a5..0b958e8544 100644 --- a/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.tsx +++ b/webapp/channels/src/components/threading/virtualized_thread_viewer/virtualized_thread_viewer.tsx @@ -44,6 +44,7 @@ type Props = { lastViewedAt: number; newMessagesSeparatorActions: PluginComponent[]; inputPlaceholder?: string; + fromSuppressed?: boolean; } type State = { @@ -357,7 +358,7 @@ class ThreadViewerVirtualized extends PureComponent { return (