Make sure the RHS does not get focused when coming back from suppressed (#26816)

* Make sure the RHS does not get focused when coming back from suppressed

* Fix missing line break

* fix tests
This commit is contained in:
Daniel Espino García 2024-04-22 12:42:54 +02:00 committed by GitHub
parent d0a67cd84a
commit 4b934d2a62
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
21 changed files with 57 additions and 167 deletions

View File

@ -636,6 +636,12 @@ export const unsuppressRHS = {
type: ActionTypes.UNSUPPRESS_RHS,
};
export function focusedRHS() {
return {
type: ActionTypes.RHS_FOCUSED,
};
}
export function setEditChannelMembers(active: boolean) {
return {
type: ActionTypes.SET_EDIT_CHANNEL_MEMBERS,

View File

@ -94,6 +94,8 @@ describe('components/AdvancedCreateComment', () => {
savePreferences(): Promise<ActionResult> {
throw new Error('Function not implemented.');
},
shouldFocusRHS: true,
focusedRHS: jest.fn(),
};
const submitEvent = {

View File

@ -174,7 +174,6 @@ export type Props = {
getChannelMemberCountsByGroup: (channelID: string) => void;
groupsWithAllowReference: Map<string, Group> | null;
channelMemberCountsByGroup: ChannelMemberCountsByGroup;
focusOnMount?: boolean;
isThreadView?: boolean;
openModal: <P>(modalData: ModalData<P>) => void;
savePreferences: (userId: string, preferences: PreferenceType[]) => Promise<ActionResult>;
@ -184,6 +183,8 @@ export type Props = {
postEditorActions: PluginComponent[];
placeholder?: string;
isPlugin?: boolean;
shouldFocusRHS: boolean;
focusedRHS: () => void;
}
type State = {
@ -276,8 +277,9 @@ class AdvancedCreateComment extends React.PureComponent<Props, State> {
onResetHistoryIndex();
setShowPreview(false);
if (this.props.focusOnMount) {
if (this.props.shouldFocusRHS) {
this.focusTextbox();
this.props.focusedRHS();
}
document.addEventListener('keydown', this.focusTextboxIfNecessary);

View File

@ -27,9 +27,11 @@ import {
} from 'actions/views/create_comment';
import {searchAssociatedGroupsForReference} from 'actions/views/group';
import {openModal} from 'actions/views/modals';
import {focusedRHS} from 'actions/views/rhs';
import {setShowPreviewOnCreateComment} from 'actions/views/textbox';
import {getCurrentLocale} from 'selectors/i18n';
import {getPostDraft, getIsRhsExpanded, getSelectedPostFocussedAt} from 'selectors/rhs';
import {getShouldFocusRHS} from 'selectors/views/rhs';
import {connectionErrorCount} from 'selectors/views/system';
import {showPreviewOnCreateComment} from 'selectors/views/textbox';
@ -79,6 +81,7 @@ function makeMapStateToProps() {
const isFormattingBarHidden = getBool(state, Constants.Preferences.ADVANCED_TEXT_EDITOR, AdvancedTextEditor.COMMENT);
const currentTeamId = getCurrentTeamId(state);
const postEditorActions = state.plugins.components.PostEditorAction;
const shouldFocusRHS = getShouldFocusRHS(state);
return {
currentTeamId,
@ -108,6 +111,7 @@ function makeMapStateToProps() {
useCustomGroupMentions,
canUploadFiles: canUploadFiles(config),
postEditorActions,
shouldFocusRHS,
};
};
}
@ -175,6 +179,7 @@ function makeMapDispatchToProps() {
openModal,
savePreferences,
searchAssociatedGroupsForReference,
focusedRHS,
},
dispatch,
);

View File

@ -30,7 +30,6 @@ exports[`components/RhsThread should match snapshot 1`] = `
rootPostId="id"
/>
<Connect(ThreadViewer)
fromSuppressed={false}
isThreadView={false}
rootPostId="id"
useRelativeTimestamp={true}

View File

@ -21,7 +21,6 @@ type Props = {
channel: Channel | null;
selected: Post | FakePost;
previousRhsState?: RhsState;
fromSuppressed: boolean;
}
const RhsThread = ({
@ -30,7 +29,6 @@ const RhsThread = ({
posts,
selected,
previousRhsState,
fromSuppressed,
}: Props) => {
const dispatch = useDispatch();
@ -61,7 +59,6 @@ const RhsThread = ({
rootPostId={selected.id}
useRelativeTimestamp={true}
isThreadView={false}
fromSuppressed={fromSuppressed}
/>
</div>
);

View File

@ -20,7 +20,6 @@ import {
getSelectedPostId,
getSelectedPostCardId,
getPreviousRhsState,
getIsRhsSuppressed,
} from 'selectors/rhs';
import {RHSStates} from 'utils/constants';
@ -42,7 +41,6 @@ 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),

View File

@ -1,74 +0,0 @@
// 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<typeof SidebarRight>;
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(<SidebarRight {...props}/>);
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();
});
});

View File

@ -28,7 +28,6 @@ import {isMac} from 'utils/user_agent';
import type {RhsState} from 'types/store/rhs';
export type Props = {
isSuppressed: boolean;
isExpanded: boolean;
isOpen: boolean;
channel: Channel;
@ -69,8 +68,6 @@ export default class SidebarRight extends React.PureComponent<Props, State> {
sidebarRightWidthHolder: React.RefObject<HTMLDivElement>;
previous: Partial<Props> | undefined = undefined;
focusSearchBar?: () => void;
lastOpenState = false;
lastSuppressedState = false;
constructor(props: Props) {
super(props);
@ -152,9 +149,6 @@ export default class SidebarRight extends React.PureComponent<Props, State> {
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);
@ -238,10 +232,7 @@ export default class SidebarRight extends React.PureComponent<Props, State> {
content = (
<div className='post-right__container'>
<FileUploadOverlay overlayType='right'/>
<RhsThread
previousRhsState={previousRhsState}
fromSuppressed={!this.lastOpenState && this.props.isOpen && this.lastSuppressedState}
/>
<RhsThread previousRhsState={previousRhsState}/>
</div>
);
} else if (postCardVisible) {

View File

@ -12,27 +12,7 @@ exports[`components/threading/ThreadViewer should match snapshot 1`] = `
overlayType="right"
/>
<DeferredRenderWrapper
channel={
Object {
"create_at": 0,
"creator_id": "",
"delete_at": 0,
"display_name": "",
"group_constrained": false,
"header": "",
"id": "channel_id",
"last_post_at": 0,
"last_root_post_at": 0,
"name": "",
"purpose": "",
"scheme_id": "",
"status": "",
"team_id": "team_id",
"teammate_id": "",
"type": "O",
"update_at": 0,
}
}
channelId="channel_id"
isThreadView={false}
key="id"
onCardClick={[Function]}

View File

@ -52,7 +52,6 @@ export type Props = Attrs & {
isThreadView: boolean;
inputPlaceholder?: string;
rootPostId: string;
fromSuppressed?: boolean;
enableWebSocketEventScope: boolean;
};
@ -225,7 +224,7 @@ export default class ThreadViewer extends React.PureComponent<Props, State> {
<DeferredThreadViewerVirt
inputPlaceholder={this.props.inputPlaceholder}
key={this.props.selected.id}
channel={this.props.channel}
channelId={this.props.channel.id}
onCardClick={this.handleCardClick}
postIds={this.props.postIds}
selected={this.props.selected}
@ -233,7 +232,6 @@ export default class ThreadViewer extends React.PureComponent<Props, State> {
highlightedPostId={this.props.highlightedPostId}
selectedPostFocusedAt={this.props.selectedPostFocusedAt}
isThreadView={Boolean(this.props.isCollapsedThreadsEnabled && this.props.isThreadView)}
fromSuppressed={this.props.fromSuppressed}
/>
)}
</>

View File

@ -21,7 +21,6 @@ import Constants from 'utils/constants';
import type {GlobalState} from 'types/store';
type Props = {
focusOnMount: boolean;
teammate?: UserProfile;
threadId: string;
latestPostId: Post['id'];
@ -30,7 +29,6 @@ type Props = {
};
const CreateComment = forwardRef<HTMLDivElement, Props>(({
focusOnMount,
teammate,
threadId,
latestPostId,
@ -98,7 +96,6 @@ const CreateComment = forwardRef<HTMLDivElement, Props>(({
>
<AdvancedCreateComment
placeholder={placeholder}
focusOnMount={focusOnMount}
channelId={channel.id}
latestPostId={latestPostId}
rootDeleted={rootDeleted}

View File

@ -3,7 +3,6 @@
import {connect} from 'react-redux';
import type {Channel} from '@mattermost/types/channels';
import type {Post} from '@mattermost/types/posts';
import {getDirectTeammate} from 'mattermost-redux/selectors/entities/channels';
@ -20,7 +19,7 @@ import type {FakePost} from 'types/store/rhs';
import ThreadViewerVirtualized from './virtualized_thread_viewer';
type OwnProps = {
channel: Channel;
channelId: string;
postIds: Array<Post['id'] | FakePost['id']>;
selected: Post | FakePost;
useRelativeTimestamp: boolean;
@ -32,12 +31,12 @@ function makeMapStateToProps() {
const getThreadLastViewedAt = makeGetThreadLastViewedAt();
return (state: GlobalState, ownProps: OwnProps) => {
const {postIds, useRelativeTimestamp, selected, channel} = ownProps;
const {postIds, useRelativeTimestamp, selected, channelId} = ownProps;
const collapsedThreads = isCollapsedThreadsEnabled(state);
const currentUserId = getCurrentUserId(state);
const lastViewedAt = getThreadLastViewedAt(state, selected.id);
const directTeammate = getDirectTeammate(state, channel.id);
const directTeammate = getDirectTeammate(state, channelId);
const lastPost = getPost(state, postIds[0]);

View File

@ -1,7 +1,6 @@
// 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';
@ -12,7 +11,6 @@ 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';
@ -32,7 +30,6 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
const directTeammate: UserProfile = TestHelper.getUserMock();
const props: Props = {
selected: post,
channel,
currentUserId: 'user_id',
directTeammate,
lastPost: post,
@ -42,7 +39,6 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
isMobileView: false,
isThreadView: false,
newMessagesSeparatorActions: [],
fromSuppressed: false,
};
const state: DeepPartial<GlobalState> = {
@ -150,31 +146,3 @@ describe('components/threading/VirtualizedThreadViewer', () => {
});
});
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(<VirtualizedThreadViewer {...props}/>, state);
expect(screen.getByRole('textbox')).toHaveFocus();
});
it('do not autofocus if fromSuppressed is set', () => {
const [props, state] = getBasePropsAndState();
props.fromSuppressed = true;
renderWithContext(<VirtualizedThreadViewer {...props}/>, state);
expect(screen.getByRole('textbox')).not.toHaveFocus();
});
});

View File

@ -7,7 +7,6 @@ import React, {PureComponent} from 'react';
import type {RefObject} from 'react';
import AutoSizer from 'react-virtualized-auto-sizer';
import type {Channel} from '@mattermost/types/channels';
import type {Post} from '@mattermost/types/posts';
import type {UserProfile} from '@mattermost/types/users';
@ -29,7 +28,6 @@ import CreateComment from './create_comment';
import Row from './thread_viewer_row';
type Props = {
channel: Channel;
currentUserId: string;
directTeammate: UserProfile | undefined;
highlightedPostId?: Post['id'];
@ -43,14 +41,12 @@ type Props = {
isThreadView: boolean;
newMessagesSeparatorActions: PluginComponent[];
inputPlaceholder?: string;
fromSuppressed?: boolean;
}
type State = {
createCommentHeight: number;
isScrolling: boolean;
topRhsPostId?: string;
userScrolled: boolean;
userScrolledToBottom: boolean;
lastViewedBottom: number;
visibleStartIndex?: number;
@ -114,7 +110,6 @@ class ThreadViewerVirtualized extends PureComponent<Props, State> {
this.state = {
createCommentHeight: 0,
isScrolling: false,
userScrolled: false,
userScrolledToBottom: false,
topRhsPostId: undefined,
lastViewedBottom: Date.now(),
@ -195,7 +190,6 @@ class ThreadViewerVirtualized extends PureComponent<Props, State> {
if (!scrollUpdateWasRequested) {
this.scrollShortCircuit = 0;
updatedState.userScrolled = true;
updatedState.userScrolledToBottom = userScrolledToBottom;
if (this.props.isMobileView) {
@ -357,7 +351,6 @@ class ThreadViewerVirtualized extends PureComponent<Props, State> {
return (
<CreateComment
placeholder={this.props.inputPlaceholder}
focusOnMount={!this.props.fromSuppressed && !this.props.isThreadView && (this.state.userScrolledToBottom || (!this.state.userScrolled && this.getInitialPostIndex() === 0))}
isThreadView={this.props.isThreadView}
latestPostId={this.props.lastPost.id}
ref={this.postCreateContainerRef}

View File

@ -28,6 +28,7 @@ describe('Reducers.RHS', () => {
isSidebarOpen: false,
isSidebarExpanded: false,
editChannelMembers: false,
shouldFocusRHS: false,
};
test('Initial state', () => {
@ -212,6 +213,7 @@ describe('Reducers.RHS', () => {
selectedPostFocussedAt: 1234,
selectedChannelId: '321',
isSidebarOpen: true,
shouldFocusRHS: true,
});
});
@ -296,6 +298,7 @@ describe('Reducers.RHS', () => {
selectedPostFocussedAt: 1234,
selectedChannelId: '321',
isSidebarOpen: true,
shouldFocusRHS: true,
});
const nextState2 = rhsReducer(
@ -317,6 +320,7 @@ describe('Reducers.RHS', () => {
selectedChannelId: '321',
previousRhsStates: [RHSStates.SEARCH],
isSidebarOpen: true,
shouldFocusRHS: true,
});
const nextState3 = rhsReducer(
@ -339,6 +343,7 @@ describe('Reducers.RHS', () => {
selectedChannelId: '321',
previousRhsStates: [RHSStates.SEARCH, RHSStates.FLAG],
isSidebarOpen: true,
shouldFocusRHS: true,
});
});
@ -419,6 +424,7 @@ describe('Reducers.RHS', () => {
selectedChannelId: '321',
previousRhsStates: [RHSStates.PIN],
isSidebarOpen: true,
shouldFocusRHS: true,
});
});
@ -619,6 +625,7 @@ describe('Reducers.RHS', () => {
isSidebarOpen: true,
isSidebarExpanded: true,
editChannelMembers: false,
shouldFocusRHS: false,
};
const nextState = rhsReducer(state, {type: ActionTypes.SUPPRESS_RHS});

View File

@ -13,7 +13,7 @@ import {
import {SidebarSize} from 'components/resizable_sidebar/constants';
import {ActionTypes, RHSStates} from 'utils/constants';
import {ActionTypes, RHSStates, Threads} from 'utils/constants';
import type {RhsState} from 'types/store/rhs';
@ -380,6 +380,21 @@ function editChannelMembers(state = false, action: AnyAction) {
}
}
function shouldFocusRHS(state = false, action: AnyAction) {
switch (action.type) {
case ActionTypes.SELECT_POST:
return Boolean(action.postId);
case Threads.CHANGED_SELECTED_THREAD:
return Boolean(action.data.thread_id);
case ActionTypes.HIGHLIGHT_REPLY:
return false;
case ActionTypes.RHS_FOCUSED:
return false;
default:
return state;
}
}
export default combineReducers({
selectedPostId,
selectedPostFocussedAt,
@ -400,4 +415,5 @@ export default combineReducers({
isSidebarExpanded,
isMenuOpen,
editChannelMembers,
shouldFocusRHS,
});

View File

@ -175,10 +175,6 @@ export function getPostDraft(state: GlobalState, prefixId: string, suffixId: str
return defaultDraft;
}
export function getIsRhsSuppressed(state: GlobalState): boolean {
return state.views.rhsSuppressed;
}
export function getIsRhsOpen(state: GlobalState): boolean {
return state.views.rhs.isSidebarOpen && !state.views.rhsSuppressed;
}

View File

@ -0,0 +1,8 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import type {GlobalState} from 'types/store';
export function getShouldFocusRHS(state: GlobalState): boolean {
return state.views.rhs.shouldFocusRHS;
}

View File

@ -41,6 +41,7 @@ export type RhsViewState = {
isMenuOpen: boolean;
editChannelMembers: boolean;
size: SidebarSize;
shouldFocusRHS: boolean;
};
export type RhsState = typeof RHSStates[keyof typeof RHSStates] | null;

View File

@ -202,6 +202,7 @@ export const ActionTypes = keyMirror({
SET_RHS_SIZE: null,
RHS_GO_BACK: null,
RHS_FOCUSED: null,
SET_RHS_EXPANDED: null,
TOGGLE_RHS_EXPANDED: null,