[MM-66586][MM-66614] Fixes for thread popouts (#34470)

* [MM-66586] Make sure current user status is manually fetched by popout controller

* [MM-66614] Add communication to allow popouts to focus reply posts, some additional fixes

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Devin Binnie
2025-11-13 16:41:51 -05:00
committed by GitHub
co-authored by Mattermost Build
parent 17ea76c408
commit bdcdff6a16
11 changed files with 173 additions and 26 deletions
@@ -13,7 +13,7 @@ let mockCanPopout = true;
jest.mock('utils/popouts/popout_windows', () => ({
__esModule: true,
get canPopout() {
canPopout: () => {
return mockCanPopout;
},
}));
@@ -20,7 +20,7 @@ export default function PopoutButton({
}: Props) {
const intl = useIntl();
if (!canPopout) {
if (!canPopout()) {
return null;
}
@@ -6,7 +6,7 @@ import React from 'react';
import {MemoryRouter} from 'react-router-dom';
import type {RouteComponentProps} from 'react-router-dom';
import {getProfiles} from 'mattermost-redux/actions/users';
import {getProfiles, getStatusesByIds} from 'mattermost-redux/actions/users';
import {renderWithContext} from 'tests/react_testing_utils';
@@ -15,6 +15,7 @@ import PopoutController from './popout_controller';
// Mock dependencies
jest.mock('mattermost-redux/actions/users', () => ({
getProfiles: jest.fn().mockReturnValue(() => ({type: 'GET_PROFILES'})),
getStatusesByIds: jest.fn().mockReturnValue(() => ({type: 'GET_STATUSES_BY_IDS'})),
}));
jest.mock('components/modal_controller', () => ({
@@ -37,6 +38,7 @@ jest.mock('components/logged_in', () => ({
}));
const mockGetProfiles = getProfiles as jest.MockedFunction<typeof getProfiles>;
const mockGetStatusesByIds = getStatusesByIds as jest.MockedFunction<typeof getStatusesByIds>;
// Base mock route props with meaningful route data
const baseRouteProps: RouteComponentProps = {
@@ -137,4 +139,23 @@ describe('PopoutController', () => {
expect(document.body.classList.contains('app__body')).toBe(true);
expect(document.body.classList.contains('popout')).toBe(true);
});
it('should dispatch getStatusesByIds with current user ID', () => {
const currentUserId = 'current-user-id-123';
const initialState = {
entities: {
users: {
currentUserId,
},
},
};
renderWithContext(
<PopoutController {...baseRouteProps}/>,
initialState,
);
expect(mockGetStatusesByIds).toHaveBeenCalledTimes(1);
expect(mockGetStatusesByIds).toHaveBeenCalledWith([currentUserId]);
});
});
@@ -2,11 +2,12 @@
// See LICENSE.txt for license information.
import React, {useEffect} from 'react';
import {useDispatch} from 'react-redux';
import {useDispatch, useSelector} from 'react-redux';
import {Route, Switch} from 'react-router-dom';
import type {RouteComponentProps} from 'react-router-dom';
import {getProfiles} from 'mattermost-redux/actions/users';
import {getProfiles, getStatusesByIds} from 'mattermost-redux/actions/users';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';
import LoggedIn from 'components/logged_in';
import ModalController from 'components/modal_controller';
@@ -19,12 +20,19 @@ import './popout_controller.scss';
const PopoutController: React.FC<RouteComponentProps> = (routeProps) => {
const dispatch = useDispatch();
const currentUserId = useSelector(getCurrentUserId);
useBrowserPopout();
useEffect(() => {
document.body.classList.add('app__body', 'popout');
dispatch(getProfiles());
}, []);
useEffect(() => {
if (currentUserId) {
dispatch(getStatusesByIds([currentUserId]));
}
}, [dispatch, currentUserId]);
return (
<LoggedIn {...routeProps}>
<ModalController/>
@@ -25,6 +25,8 @@ import {
import {getIsRhsExpanded} from 'selectors/rhs';
import {getIsMobileView} from 'selectors/views/browser';
import {focusPost} from 'components/permalink_view/actions';
import {CrtThreadPaneSteps, Preferences} from 'utils/constants';
import {matchUserMentionTriggersWithMessageMentions} from 'utils/post_utils';
import {allAtMentions} from 'utils/text_formatting';
@@ -84,6 +86,7 @@ const actions = {
toggleRhsExpanded,
setThreadFollow,
goBack,
focusPost,
};
export default connect(makeMapStateToProps, actions)(RhsHeaderPost);
@@ -43,6 +43,7 @@ type Props = WrappedComponentProps & {
closeRightHandSide: (e?: React.MouseEvent) => void;
toggleRhsExpanded: (e: React.MouseEvent) => void;
setThreadFollow: (userId: string, teamId: string, threadId: string, newState: boolean) => void;
focusPost: (postId: string, returnTo: string, currentUserId: string, option?: {skipRedirectReplyPermalink: boolean}) => Promise<void>;
};
class RhsHeaderPost extends React.PureComponent<Props> {
@@ -79,11 +80,14 @@ class RhsHeaderPost extends React.PureComponent<Props> {
this.props.setThreadFollow(currentUserId, currentTeam.id, rootPostId, !isFollowingThread);
};
popout = () => {
if (!this.props.currentTeam) {
popout = async () => {
const {currentTeam, intl, rootPostId, focusPost, currentUserId} = this.props;
if (!currentTeam) {
return;
}
popoutThread(this.props.intl, this.props.rootPostId, this.props.currentTeam.name);
await popoutThread(intl, rootPostId, currentTeam.name, (postId, returnTo) => {
focusPost(postId, returnTo, currentUserId, {skipRedirectReplyPermalink: true});
});
};
render() {
@@ -17,6 +17,7 @@ import {
} from 'actions/post_actions';
import {manuallyMarkThreadAsUnread} from 'actions/views/threads';
import {focusPost} from 'components/permalink_view/actions';
import Menu from 'components/widgets/menu/menu';
import MenuWrapper from 'components/widgets/menu/menu_wrapper';
@@ -87,8 +88,10 @@ function ThreadMenu({
]);
const popout = useCallback(() => {
popoutThread(intl, threadId, team);
}, [threadId, team, intl]);
popoutThread(intl, threadId, team, (postId, returnTo) => {
dispatch(focusPost(postId, returnTo, currentUserId, {skipRedirectReplyPermalink: true}));
});
}, [threadId, team, intl, dispatch, currentUserId]);
return (
<MenuWrapper
@@ -102,7 +105,7 @@ function ThreadMenu({
})}
openLeft={true}
>
{!canPopout && (
{!canPopout() && (
<Menu.ItemAction
buttonClass='PopoutMenuItem'
text={formatMessage({
@@ -13,6 +13,7 @@ import {setThreadFollow} from 'mattermost-redux/actions/threads';
import {makeGetChannel} from 'mattermost-redux/selectors/entities/channels';
import {getPost, makeGetPostsForThread} from 'mattermost-redux/selectors/entities/posts';
import {focusPost} from 'components/permalink_view/actions';
import PopoutButton from 'components/popout_button';
import Header from 'components/widgets/header';
import WithTooltip from 'components/with_tooltip';
@@ -82,8 +83,10 @@ const ThreadPane = ({
}, [dispatch, currentUserId, currentTeamId, threadId, isFollowing]);
const popout = useCallback(() => {
popoutThread(intl, threadId, team);
}, [threadId, team, intl]);
popoutThread(intl, threadId, team, (postId, returnTo) => {
dispatch(focusPost(postId, returnTo, currentUserId, {skipRedirectReplyPermalink: true}));
});
}, [threadId, team, intl, dispatch, currentUserId]);
return (
<div
@@ -6,7 +6,7 @@ import type {IntlShape} from 'react-intl';
import DesktopApp from 'utils/desktop_api';
import {isDesktopApp} from 'utils/user_agent';
import {popoutThread} from './popout_windows';
import {FOCUS_REPLY_POST, popoutThread} from './popout_windows';
// Mock dependencies
jest.mock('utils/desktop_api', () => ({
@@ -22,9 +22,24 @@ jest.mock('utils/user_agent', () => ({
isDesktopApp: jest.fn(),
}));
jest.mock('./browser_popouts', () => {
const mockFn = jest.fn();
(globalThis as typeof globalThis & {mockSetupBrowserPopout: typeof mockFn}).mockSetupBrowserPopout = mockFn;
return {
__esModule: true,
default: {
setupBrowserPopout: mockFn,
},
};
});
const mockDesktopApp = DesktopApp as jest.Mocked<typeof DesktopApp>;
const mockIsDesktopApp = isDesktopApp as jest.MockedFunction<typeof isDesktopApp>;
const getMockSetupBrowserPopout = () => {
return (globalThis as typeof globalThis & {mockSetupBrowserPopout: jest.MockedFunction<() => unknown>}).mockSetupBrowserPopout;
};
describe('popout_windows', () => {
const mockIntl = {
formatMessage: jest.fn(({id, defaultMessage}) => {
@@ -37,12 +52,26 @@ describe('popout_windows', () => {
beforeEach(() => {
jest.clearAllMocks();
getMockSetupBrowserPopout().mockClear();
});
describe('popoutThread', () => {
it('should call popout with correct path and props', async () => {
const mockOnFocusPost = jest.fn();
beforeEach(() => {
mockOnFocusPost.mockClear();
});
it('should call popout with correct path and props for desktop app', async () => {
mockIsDesktopApp.mockReturnValue(true);
await popoutThread(mockIntl, 'thread-123', 'test-team');
const mockListeners = {
sendToPopout: jest.fn(),
onMessageFromPopout: jest.fn(),
onClosePopout: jest.fn(),
};
mockDesktopApp.setupDesktopPopout.mockResolvedValue(mockListeners);
await popoutThread(mockIntl, 'thread-123', 'test-team', mockOnFocusPost);
expect(mockDesktopApp.setupDesktopPopout).toHaveBeenCalledWith(
'/_popout/thread/test-team/thread-123',
@@ -53,7 +82,23 @@ describe('popout_windows', () => {
);
});
it('should handle desktop app popout', async () => {
it('should call popout with correct path and props for browser popout', async () => {
mockIsDesktopApp.mockReturnValue(false);
const mockListeners = {
sendToPopout: jest.fn(),
onMessageFromPopout: jest.fn(),
onClosePopout: jest.fn(),
};
getMockSetupBrowserPopout().mockReturnValue(mockListeners);
await popoutThread(mockIntl, 'thread-123', 'test-team', mockOnFocusPost);
expect(getMockSetupBrowserPopout()).toHaveBeenCalledWith(
'/_popout/thread/test-team/thread-123',
);
});
it('should return popout listeners', async () => {
mockIsDesktopApp.mockReturnValue(true);
const mockListeners = {
sendToPopout: jest.fn(),
@@ -62,10 +107,31 @@ describe('popout_windows', () => {
};
mockDesktopApp.setupDesktopPopout.mockResolvedValue(mockListeners);
const result = await popoutThread(mockIntl, 'thread-123', 'test-team');
const result = await popoutThread(mockIntl, 'thread-123', 'test-team', mockOnFocusPost);
expect(result).toEqual(mockListeners);
});
it('should set up listener for FOCUS_REPLY_POST messages', async () => {
mockIsDesktopApp.mockReturnValue(true);
const mockListener = jest.fn();
const mockListeners = {
sendToPopout: jest.fn(),
onMessageFromPopout: mockListener,
onClosePopout: jest.fn(),
};
mockDesktopApp.setupDesktopPopout.mockResolvedValue(mockListeners);
await popoutThread(mockIntl, 'thread-123', 'test-team', mockOnFocusPost);
expect(mockListener).toHaveBeenCalledTimes(1);
const registeredListener = mockListener.mock.calls[0][0];
registeredListener(FOCUS_REPLY_POST, 'post-123', '/team/pl/post-123');
expect(mockOnFocusPost).toHaveBeenCalledTimes(1);
expect(mockOnFocusPost).toHaveBeenCalledWith('post-123', '/team/pl/post-123');
});
});
});
@@ -5,22 +5,46 @@ import type {IntlShape} from 'react-intl';
import type {PopoutViewProps} from '@mattermost/desktop-api';
import {Client4} from 'mattermost-redux/client';
import DesktopApp from 'utils/desktop_api';
import {isDesktopApp} from 'utils/user_agent';
import BrowserPopouts from './browser_popouts';
import {sendToParent as sendToParentBrowser, onMessageFromParent as onMessageFromParentBrowser} from './use_browser_popout';
import {
sendToParent as sendToParentBrowser,
onMessageFromParent as onMessageFromParentBrowser,
} from './use_browser_popout';
export const canPopout = Boolean(!isDesktopApp() || DesktopApp.canPopout());
export function popoutThread(intl: IntlShape, threadId: string, teamName: string) {
return popout(
export const FOCUS_REPLY_POST = 'focus-reply-post';
export async function popoutThread(
intl: IntlShape,
threadId: string,
teamName: string,
onFocusPost: (postId: string, returnTo: string) => void,
) {
const popoutListeners = await popout(
`/_popout/thread/${teamName}/${threadId}`,
{
isRHS: true,
titleTemplate: intl.formatMessage({id: 'thread_popout.title', defaultMessage: 'Thread - {channelName} - {teamName}'}),
titleTemplate: intl.formatMessage({
id: 'thread_popout.title',
defaultMessage: 'Thread - {channelName} - {teamName}',
}),
},
);
popoutListeners?.onMessageFromPopout?.((channel: string, ...args: unknown[]) => {
if (channel === FOCUS_REPLY_POST) {
const [postId, returnTo] = args;
onFocusPost(
postId as string,
returnTo as string,
);
}
});
return popoutListeners;
}
/**
@@ -34,7 +58,10 @@ type PopoutListeners = {
onClosePopout: (listener: () => void) => void;
};
async function popout(path: string, desktopProps?: PopoutViewProps): Promise<Partial<PopoutListeners>> {
async function popout(
path: string,
desktopProps?: PopoutViewProps,
): Promise<Partial<PopoutListeners>> {
if (isDesktopApp()) {
return DesktopApp.setupDesktopPopout(path, desktopProps);
}
@@ -58,3 +85,10 @@ export function onMessageFromParent(listener: (channel: string, ...args: unknown
return onMessageFromParentBrowser(listener);
}
export function isPopoutWindow() {
return window.location.href.startsWith(`${Client4.getUrl()}/_popout/`);
}
export function canPopout() {
return Boolean(!isDesktopApp() || DesktopApp.canPopout());
}
+6 -1
View File
@@ -58,6 +58,7 @@ import Constants, {FileTypes, ValidationErrors, A11yCustomEventTypes, AdvancedTe
import type {A11yFocusEventDetail} from 'utils/constants';
import DesktopApp from 'utils/desktop_api';
import * as Keyboard from 'utils/keyboard';
import {FOCUS_REPLY_POST, isPopoutWindow, sendToParent} from 'utils/popouts/popout_windows';
import * as UserAgent from 'utils/user_agent';
import {joinPrivateChannelPrompt} from './channel_utils';
@@ -1377,7 +1378,11 @@ export async function handleFormattedTextClick(e: React.UIEvent, currentRelative
e.stopPropagation();
if (match && match.type === 'permalink' && isTeamSameWithCurrentTeam(state, match.teamName) && isReply && crtEnabled) {
store.dispatch(focusPost(match.postId ?? '', linkAttribute.value, user.id, {skipRedirectReplyPermalink: true}));
if (isPopoutWindow()) {
sendToParent(FOCUS_REPLY_POST, match.postId ?? '', linkAttribute.value);
} else {
store.dispatch(focusPost(match.postId ?? '', linkAttribute.value, user.id, {skipRedirectReplyPermalink: true}));
}
} else {
getHistory().push(linkAttribute.value);
}