From f9fb13c7e6da9a07c9d6d38cf4b57396a3045ea9 Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Wed, 10 Dec 2025 07:17:54 +0200 Subject: [PATCH] [MM-65186] Keyboard focus is wrong when using Shift-Up to reply in thread (#34627) (#34697) (cherry picked from commit 4fb41f3ba274f32b8e23f52e46b82e3d93f63af6) Co-authored-by: M-ZubairAhmed --- .../keyboard_shortcuts_1_spec.js | 4 +++- .../use_textbox_focus.tsx | 24 ++++++++++++------- .../sidebar_right/sidebar_right.tsx | 6 +++++ webapp/channels/src/utils/user_agent.tsx | 2 +- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/e2e-tests/cypress/tests/integration/channels/keyboard_shortcuts/keyboard_shortcuts_1_spec.js b/e2e-tests/cypress/tests/integration/channels/keyboard_shortcuts/keyboard_shortcuts_1_spec.js index 92ace315fd..6e0f79e1a8 100644 --- a/e2e-tests/cypress/tests/integration/channels/keyboard_shortcuts/keyboard_shortcuts_1_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/keyboard_shortcuts/keyboard_shortcuts_1_spec.js @@ -11,7 +11,7 @@ // Group: @channels @keyboard_shortcuts import * as messages from '../../../fixtures/messages'; -import * as TIMEOUTS from '../../../fixtures/timeouts'; +import timeouts, * as TIMEOUTS from '../../../fixtures/timeouts'; describe('Keyboard Shortcuts', () => { let testTeam; @@ -377,6 +377,7 @@ describe('Keyboard Shortcuts', () => { cy.get('body').cmdOrCtrlShortcut('{shift}L'); cy.uiGetPostTextBox().should('be.focused'); + cy.get('[data-testid="searchBoxClose"] > .icon').click(); // # Post a message and open RHS const message = `hello${Date.now()}`; cy.postMessage(message); @@ -386,6 +387,7 @@ describe('Keyboard Shortcuts', () => { cy.uiGetReplyTextBox().focus().should('be.focused'); }).then(() => { // # Type CTRL/CMD+SHIFT+L + cy.wait(timeouts.ONE_SEC); cy.get('body').cmdOrCtrlShortcut('{shift}L'); cy.uiGetPostTextBox().should('be.focused'); }); diff --git a/webapp/channels/src/components/advanced_text_editor/use_textbox_focus.tsx b/webapp/channels/src/components/advanced_text_editor/use_textbox_focus.tsx index aaefb654ee..c790830d5a 100644 --- a/webapp/channels/src/components/advanced_text_editor/use_textbox_focus.tsx +++ b/webapp/channels/src/components/advanced_text_editor/use_textbox_focus.tsx @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import type React from 'react'; -import {useCallback, useEffect} from 'react'; +import {useCallback, useEffect, useRef} from 'react'; import {useDispatch, useSelector} from 'react-redux'; import {focusedRHS} from 'actions/views/rhs'; @@ -23,11 +23,10 @@ const useTextboxFocus = ( ) => { const dispatch = useDispatch(); + const hasMounted = useRef(false); + const rhsExpanded = useSelector(getIsRhsExpanded); const rhsOpen = useSelector(getIsRhsOpen); - - // We force the selector to always think it is the same value to avoid re-renders - // because we only use this value during mount. const shouldFocusRHS = useSelector(getShouldFocusRHS, () => true); const focusTextbox = useCallback((keepFocus = false) => { @@ -48,7 +47,7 @@ const useTextboxFocus = ( textboxRef.current?.focus(); }); } - }, [canPost, textboxRef]); + }, [canPost]); const focusTextboxIfNecessary = useCallback((e: KeyboardEvent) => { // Do not focus if the rhs is expanded and this is not the RHS @@ -91,15 +90,24 @@ const useTextboxFocus = ( focusTextbox(); }, [channelId]); - // Focus on mount useEffect(() => { if (isRHS && shouldFocusRHS) { + // If we are in the RHS and we are supposed to focus the RHS because of a reply, + // we focus the textbox and reset the shouldFocusRHS flag. focusTextbox(); dispatch(focusedRHS()); - } else if (!isRHS && !shouldFocusRHS) { + } else if (!isRHS && !shouldFocusRHS && !hasMounted.current) { + // If we are in the Center channel and we are not supposed to focus the RHS, + // we focus the textbox but only on mount. + // This is because if we focus on updates, we might steal focus from the RHS + // when the RHS focuses and resets the shouldFocusRHS flag. focusTextbox(); } - }, []); + + if (!hasMounted.current) { + hasMounted.current = true; + } + }, [isRHS, shouldFocusRHS, focusTextbox, dispatch]); return focusTextbox; }; diff --git a/webapp/channels/src/components/sidebar_right/sidebar_right.tsx b/webapp/channels/src/components/sidebar_right/sidebar_right.tsx index 141be221cd..d324ca54d3 100644 --- a/webapp/channels/src/components/sidebar_right/sidebar_right.tsx +++ b/webapp/channels/src/components/sidebar_right/sidebar_right.tsx @@ -160,6 +160,12 @@ export default class SidebarRight extends React.PureComponent { if (this.props.isOpen && (contentChanged || (!wasOpen && isOpen))) { this.previousActiveElement = document.activeElement as HTMLElement; + // For RHS with textbox, don't auto-focus the first element with this approach. + // The RHS textbox will focus itself via use_textbox_focus.tsx hook with correct focus logic. + if (this.props.postRightVisible) { + return; + } + // Focus the sidebar after a tick setTimeout(() => { if (this.sidebarRight.current) { diff --git a/webapp/channels/src/utils/user_agent.tsx b/webapp/channels/src/utils/user_agent.tsx index 15da2d1a31..296d9e74ba 100644 --- a/webapp/channels/src/utils/user_agent.tsx +++ b/webapp/channels/src/utils/user_agent.tsx @@ -105,7 +105,7 @@ export function isMobileApp(): boolean { return isMobile() && !isIosWeb() && !isAndroidWeb(); } -// Returns true if and only if the user is using Mattermost from either the mobile app or the web browser on a mobile device. +// Returns true if and only if the user is using Mattermost from the web browser on a mobile device. export function isMobile(): boolean { return isIos() || isAndroid(); }