From 2fb38fe71d7b5d34885dd7d38915c0ff929048e1 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Mon, 13 Apr 2026 14:36:17 -0400 Subject: [PATCH] [MM-68266] Pass through menu props to popout menu item, guard at menu definition to avoid null component blocking keyboard navigation (#36024) * [MM-68266] Pass through menu props to popout menu item, guard at menu definition to avoid null component blocking keyboard navigation * fix tests * Update webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/sidebar_channel_menu.test.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../channel_header_menu.tsx | 5 ++- .../menu_items/open_in_new_window.test.tsx | 11 +++-- .../menu_items/open_in_new_window.tsx | 13 +++--- .../src/components/popout_menu_item.tsx | 9 ++--- .../sidebar_channel_menu.test.tsx.snap | 40 +++++++++---------- .../sidebar_channel_menu.test.tsx | 19 +++++++++ .../sidebar_channel_menu.tsx | 5 ++- 7 files changed, 61 insertions(+), 41 deletions(-) diff --git a/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx b/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx index 3b0c34b08a9..81d017df2aa 100644 --- a/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx +++ b/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx @@ -28,6 +28,7 @@ import {getIsChannelBookmarksEnabled} from 'components/channel_bookmarks/utils'; import * as Menu from 'components/menu'; import {Constants} from 'utils/constants'; +import {canPopout, isChannelPopoutWindow} from 'utils/popouts/popout_windows'; import type {GlobalState} from 'types/store'; @@ -149,7 +150,9 @@ export default function ChannelHeaderMenu({dmUser, gmMembers, isMobile, archived horizontal: 'left', }} > - + {canPopout() && !isChannelPopoutWindow() && ( + + )} {isDirect && ( { beforeEach(() => { jest.clearAllMocks(); - jest.mocked(isChannelPopoutWindow).mockReturnValue(false); }); - test('should render nothing when already in a channel popout', () => { - jest.mocked(isChannelPopoutWindow).mockReturnValue(true); + test('should render menu item and separator', () => { const channel = TestHelper.getChannelMock({type: 'O' as ChannelType, name: 'town-square'}); - const {container} = renderWithContext( + renderWithContext( , baseState, ); - expect(container).toBeEmptyDOMElement(); + expect(screen.getByText('Open in new window')).toBeInTheDocument(); + expect(screen.getByRole('separator')).toBeInTheDocument(); }); test('should call popoutChannel when clicked', async () => { diff --git a/webapp/channels/src/components/channel_header_menu/menu_items/open_in_new_window.tsx b/webapp/channels/src/components/channel_header_menu/menu_items/open_in_new_window.tsx index beb47e741e9..b7a03fa0973 100644 --- a/webapp/channels/src/components/channel_header_menu/menu_items/open_in_new_window.tsx +++ b/webapp/channels/src/components/channel_header_menu/menu_items/open_in_new_window.tsx @@ -13,19 +13,19 @@ import {getUserIdFromChannelName} from 'mattermost-redux/utils/channel_utils'; import {getPopoutChannelTitle} from 'components/channel_popout/channel_popout'; import * as Menu from 'components/menu'; -import PopoutMenuItem from 'components/popout_menu_item'; +import PopoutMenuItem, {type PopoutMenuItemProps} from 'components/popout_menu_item'; import {getChannelRoutePathAndIdentifier} from 'utils/channel_utils'; import {Constants} from 'utils/constants'; -import {isChannelPopoutWindow, popoutChannel} from 'utils/popouts/popout_windows'; +import {popoutChannel} from 'utils/popouts/popout_windows'; import type {GlobalState} from 'types/store'; -interface Props { +interface Props extends PopoutMenuItemProps { channel: Channel; } -const MenuItemOpenInNewWindow = ({channel}: Props) => { +const MenuItemOpenInNewWindow = ({channel, ...rest}: Props) => { const intl = useIntl(); const team = useSelector(getCurrentTeam); const currentUserId = useSelector(getCurrentUserId); @@ -37,10 +37,6 @@ const MenuItemOpenInNewWindow = ({channel}: Props) => { return undefined; }); - if (isChannelPopoutWindow()) { - return null; - } - const handleClick = () => { if (!team) { return; @@ -55,6 +51,7 @@ const MenuItemOpenInNewWindow = ({channel}: Props) => { diff --git a/webapp/channels/src/components/popout_menu_item.tsx b/webapp/channels/src/components/popout_menu_item.tsx index 5e263a7603b..3a214e92ee4 100644 --- a/webapp/channels/src/components/popout_menu_item.tsx +++ b/webapp/channels/src/components/popout_menu_item.tsx @@ -7,15 +7,13 @@ import {FormattedMessage} from 'react-intl'; import {DockWindowIcon} from '@mattermost/compass-icons/components'; import * as Menu from 'components/menu'; +import type {Props as MenuItemProps} from 'components/menu/menu_item'; import {canPopout} from 'utils/popouts/popout_windows'; -type Props = { - onClick: () => void; - id?: string; -}; +export type PopoutMenuItemProps = Omit; -export default function PopoutMenuItem({onClick, id = 'openInNewWindow'}: Props) { +export default function PopoutMenuItem({onClick, id = 'openInNewWindow', ...rest}: PopoutMenuItemProps) { if (!canPopout()) { return null; } @@ -31,6 +29,7 @@ export default function PopoutMenuItem({onClick, id = 'openInNewWindow'}: Props) /> } onClick={onClick} + {...rest} /> ); } diff --git a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/__snapshots__/sidebar_channel_menu.test.tsx.snap b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/__snapshots__/sidebar_channel_menu.test.tsx.snap index eec4e3a4a96..d7c4971e136 100644 --- a/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/__snapshots__/sidebar_channel_menu.test.tsx.snap +++ b/webapp/channels/src/components/sidebar/sidebar_channel/sidebar_channel_menu/__snapshots__/sidebar_channel_menu.test.tsx.snap @@ -108,10 +108,10 @@ exports[`components/sidebar/sidebar_channel/sidebar_channel_menu should match sn tabindex="-1" >