From 02bae8c3a1caaacc435267c00aeee31a42fe6a1b Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 21 May 2026 10:33:56 +0200 Subject: [PATCH] Fix: Global Threads view shows only 1 quick reaction emoji instead of 3 (MM-68681) (#36512) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: Global Threads shows 1 quick reaction emoji instead of 3 (MM-68681) When posts are viewed in the Global Threads full-width view, the hover toolbar was incorrectly showing only 1 quick reaction emoji instead of 3. Root cause: In post/index.tsx, the isExpanded prop (which controls whether the toolbar shows 3 or 1 emojis) was derived only from state.views.rhs.isSidebarExpanded. When navigating to Global Threads, suppressRHS is dispatched (setting state.views.rhsSuppressed = true), but isSidebarExpanded remains false. Since posts in the thread viewer use RHS_ROOT/RHS_COMMENT locations (not CENTER), and the #sidebar-right element is suppressed (width = 0), the showMoreReactions check in post_options.tsx always fell through to showing only 1 emoji. Fix: Include state.views.rhsSuppressed in the isExpanded computation so that when the RHS is suppressed (i.e., we are in a full-width context like Global Threads or Drafts), the toolbar correctly renders 3 quick reaction emojis. Tests: Added post_options.test.tsx with 4 unit tests verifying: - CENTER location → 3 emojis (existing behavior) - RHS_ROOT + isExpanded=false → 1 emoji (narrow RHS, existing behavior) - RHS_ROOT + isExpanded=true → 3 emojis (expanded RHS or Global Threads) - RHS_COMMENT + isExpanded=true → 3 emojis Co-authored-by: Miguel de la Cruz * Refactor: use getIsGlobalThreadsView selector instead of rhsSuppressed (MM-68681) Replace the broad state.views.rhsSuppressed check (which also fires for the Drafts view) with a precise getIsGlobalThreadsView() selector that reads the already-existing state.views.lhs.currentStaticPageId field. When global_threads.tsx mounts it dispatches selectLhsItem(LhsItemType.Page, LhsPage.Threads) which sets currentStaticPageId to LhsPage.Threads ('threads'). This is the existing, canonical signal that the Global Threads full-width view is active; the selector wraps it with a name that conveys intent directly. Changes: - selectors/lhs.ts: add getIsGlobalThreadsView() — returns true iff state.views.lhs.currentStaticPageId === LhsPage.Threads - selectors/lhs.test.ts: 3 new tests covering Threads, Drafts, and empty page - post/index.tsx: import getIsGlobalThreadsView and use it in isExpanded - post_options.test.tsx: update test description to match new mechanism Co-authored-by: Miguel de la Cruz * Fix tests: use renderWithContext + real component tree, not jest.mock The previous post_options.test.tsx used jest.mock() to replace PostRecentReactions, DotMenu, and several other components. That pattern is not established in this codebase — post_component.test.tsx and post_reaction.test.tsx both exercise the real connected component tree via renderWithContext with a partial Redux state. Rewrite to match: - Drop all jest.mock() calls for child components. - Provide minimal Redux state (roles with ADD_REACTION + user with system_user role) so that ChannelPermissionGate lets the emoji buttons render — the same pattern used in post_reaction.test.tsx. - Use proper SystemEmoji shaped objects (with short_name) as recentEmojis so that getEmojiName() does not crash. - Assert on the real rendered emoji buttons (data-testid= 'post-menu__item_emoji') rather than a mocked prop capture. Co-authored-by: Miguel de la Cruz * Remove selector comment; add e2e test for Global Threads quick reactions (MM-68681) - selectors/lhs.ts: remove JSDoc block from getIsGlobalThreadsView; the name is self-explanatory and the comment was narrating the code. - emoji_recently_used_spec.js: * Add group tag @collapsed_reply_threads (test now requires CRT config). * Add MM-T4261_3: verifies that hovering a post in the Global Threads full-width panel shows 3 quick reaction emojis, matching the center channel and unlike the narrow RHS sidebar which shows 1. * Add GLOBAL_THREADS case to validateQuickReactions helper (uses rhsPost id prefix, numReactions=3). Co-authored-by: Miguel de la Cruz * Fix lint: correct import order in post_options.test.tsx and post/index.tsx - post_options.test.tsx: move @mattermost/types/emojis type import before mattermost-redux/constants; add missing blank line between import groups. - post/index.tsx: move selectors/lhs import before selectors/posts (alphabetical order within the selectors/* group). Co-authored-by: Miguel de la Cruz * Fix types: use correct EmojiCategory value 'people-body' not 'people' Co-authored-by: Miguel de la Cruz * Remove dead RHS_EXPANDED branch from validateQuickReactions helper No call site ever passes 'RHS_EXPANDED' to validateQuickReactions — the branch was unreachable. Keep only the 'GLOBAL_THREADS' case that the new MM-T4261_3 test actually exercises. Co-authored-by: Miguel de la Cruz --------- Co-authored-by: Cursor Agent Co-authored-by: Miguel de la Cruz --- .../messaging/emoji_recently_used_spec.js | 55 +++++++- webapp/channels/src/components/post/index.tsx | 3 +- .../src/components/post/post_options.test.tsx | 127 ++++++++++++++++++ webapp/channels/src/selectors/lhs.test.ts | 36 +++++ webapp/channels/src/selectors/lhs.ts | 5 + 5 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 webapp/channels/src/components/post/post_options.test.tsx diff --git a/e2e-tests/cypress/tests/integration/channels/messaging/emoji_recently_used_spec.js b/e2e-tests/cypress/tests/integration/channels/messaging/emoji_recently_used_spec.js index 2899f5364d3..cae4c1c07a2 100644 --- a/e2e-tests/cypress/tests/integration/channels/messaging/emoji_recently_used_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/messaging/emoji_recently_used_spec.js @@ -8,7 +8,7 @@ // *************************************************************** // Stage: @prod -// Group: @channels @messaging +// Group: @channels @messaging @collapsed_reply_threads import timeouts from '@/fixtures/timeouts'; @@ -177,6 +177,57 @@ describe('Messaging', () => { }); }); + it('MM-T4261_3 One-Click Reactions in Global Threads view show 3 emojis', () => { + // # Re-enable emoji picker (MM-T4261_2 disables it) and configure CRT server-side + cy.apiAdminLogin(); + cy.apiUpdateConfig({ + ServiceSettings: { + EnableEmojiPicker: true, + ThreadAutoFollow: true, + CollapsedThreads: 'default_off', + }, + }); + + // # Create a fresh user with CRT turned on so threads appear in the Threads view + cy.apiCreateUser({prefix: 'crtUser'}).then(({user: crtUser}) => { + cy.apiAddUserToTeam(testTeam.id, crtUser.id); + cy.apiSaveCRTPreference(crtUser.id, 'on'); + cy.apiLogin(crtUser); + }); + + cy.visit(offTopicPath); + + // # Enable one-click reactions for this user + cy.uiOpenSettingsModal('Display').within(() => { + cy.findByText('Display', {timeout: timeouts.ONE_MIN}).click(); + cy.findByText('Quick reactions on messages').click(); + cy.findByLabelText('On').click(); + cy.uiSaveAndClose(); + }); + + // # Post a root message and a reply so a followed thread exists + cy.apiGetChannelByName(testTeam.name, 'off-topic').then(({channel}) => { + cy.apiCreatePost(channel.id, 'Root post for Global Threads emoji test', '', {}).then((rootResp) => { + const rootPostId = rootResp.body.id; + + cy.apiCreatePost(channel.id, 'Reply to follow the thread', rootPostId, {}); + + // # Navigate to Global Threads + cy.uiClickSidebarItem('threads'); + + // # Click the thread to open the full-width thread panel + cy.get('div.ThreadItem').should('have.lengthOf.at.least', 1).first().click(); + + // * Root post is visible in the thread pane + cy.get(`#rhsPost_${rootPostId}`).should('be.visible'); + + // * Hovering over the post in Global Threads shows 3 quick reaction emojis — + // the same count as the center channel, not the 1 shown in a narrow RHS sidebar. + validateQuickReactions(rootPostId, 'GLOBAL_THREADS', defaultEmojis); + }); + }); + }); + function validateQuickReactions(postId, location, emojis) { let idPrefix; let numReactions = 3; @@ -186,7 +237,7 @@ describe('Messaging', () => { } else if (location === 'RHS_ROOT' || location === 'RHS_COMMENT') { idPrefix = 'rhsPost'; numReactions = 1; - } else if (location === 'RHS_EXPANDED') { + } else if (location === 'GLOBAL_THREADS') { idPrefix = 'rhsPost'; } diff --git a/webapp/channels/src/components/post/index.tsx b/webapp/channels/src/components/post/index.tsx index 7a4c6a9fbab..4eac3dde910 100644 --- a/webapp/channels/src/components/post/index.tsx +++ b/webapp/channels/src/components/post/index.tsx @@ -32,6 +32,7 @@ import {closeRightHandSide, selectPost, setRhsExpanded, selectPostCard, selectPo import {getBurnOnReadDurationMinutes} from 'selectors/burn_on_read'; import {isBurnOnReadPost, shouldDisplayConcealedPlaceholder} from 'selectors/burn_on_read_posts'; import {getShortcutReactToLastPostEmittedFrom, getOneClickReactionEmojis} from 'selectors/emojis'; +import {getIsGlobalThreadsView} from 'selectors/lhs'; import {getIsPostBeingEdited, getIsPostBeingEditedInRHS, isEmbedVisible} from 'selectors/posts'; import {getHighlightedPostId, getRhsState, getSelectedPostCard} from 'selectors/rhs'; import {getIsMobileView} from 'selectors/views/browser'; @@ -219,7 +220,7 @@ function makeMapStateToProps() { recentEmojis: emojis, center: get(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.CHANNEL_DISPLAY_MODE, Preferences.CHANNEL_DISPLAY_MODE_DEFAULT) === Preferences.CHANNEL_DISPLAY_MODE_CENTERED, isCollapsedThreadsEnabled: isCollapsedThreadsEnabled(state), - isExpanded: state.views.rhs.isSidebarExpanded, + isExpanded: state.views.rhs.isSidebarExpanded || getIsGlobalThreadsView(state), isPostBeingEdited: ownProps.location === Locations.CENTER ? !getIsPostBeingEditedInRHS(state, post.id) && getIsPostBeingEdited(state, post.id) : getIsPostBeingEditedInRHS(state, post.id), isMobileView: getIsMobileView(state), previewCollapsed, diff --git a/webapp/channels/src/components/post/post_options.test.tsx b/webapp/channels/src/components/post/post_options.test.tsx new file mode 100644 index 00000000000..704e306b6f6 --- /dev/null +++ b/webapp/channels/src/components/post/post_options.test.tsx @@ -0,0 +1,127 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import type {SystemEmoji} from '@mattermost/types/emojis'; + +import {Permissions} from 'mattermost-redux/constants'; + +import {renderWithContext, screen} from 'tests/react_testing_utils'; +import {Locations} from 'utils/constants'; +import {TestHelper} from 'utils/test_helper'; + +import PostOptions from './post_options'; + +// Minimal Redux state: grant ADD_REACTION to the current user so that +// ChannelPermissionGate lets the quick-reaction emoji buttons render. +const currentUserId = 'currentUser'; +const channel = TestHelper.getChannelMock({team_id: 'team1'}); +const baseState = { + entities: { + roles: { + roles: { + system_user: TestHelper.getRoleMock({permissions: [Permissions.ADD_REACTION]}), + }, + }, + users: { + currentUserId, + profiles: { + [currentUserId]: TestHelper.getUserMock({id: currentUserId, roles: 'system_user'}), + }, + }, + }, +}; + +// Proper SystemEmoji shapes — getEmojiName() reads `short_name` for system emojis. +const makeSystemEmoji = (shortName: string): SystemEmoji => ({ + name: shortName, + short_name: shortName, + short_names: [shortName], + category: 'people-body', + unified: shortName.toUpperCase(), +}); + +const post = TestHelper.getPostMock({type: '', channel_id: channel.id}); + +const baseProps = { + post, + teamId: channel.team_id, + isFlagged: false, + removePost: jest.fn(), + enableEmojiPicker: true, + isReadOnly: false, + channelIsArchived: false, + handleDropdownOpened: jest.fn(), + oneClickReactionsEnabled: true, + recentEmojis: [ + makeSystemEmoji('thumbsup'), + makeSystemEmoji('grinning'), + makeSystemEmoji('white_check_mark'), + ], + isMobileView: false, + location: Locations.RHS_ROOT as keyof typeof Locations, + pluginActions: [], + isChannelAutotranslated: false, + actions: { + emitShortcutReactToLastPostFrom: jest.fn(), + }, +}; + +describe('PostOptions - quick reaction count (MM-68681)', () => { + test('CENTER location always shows 3 quick reaction emojis', () => { + renderWithContext( + , + baseState, + ); + + expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3); + }); + + test('RHS_ROOT with isExpanded false (narrow sidebar) shows 1 quick reaction emoji', () => { + renderWithContext( + , + baseState, + ); + + expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(1); + }); + + test('RHS_ROOT with isExpanded true (expanded sidebar or Global Threads view) shows 3 quick reaction emojis', () => { + renderWithContext( + , + baseState, + ); + + expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3); + }); + + test('RHS_COMMENT with isExpanded true shows 3 quick reaction emojis', () => { + renderWithContext( + , + baseState, + ); + + expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3); + }); +}); diff --git a/webapp/channels/src/selectors/lhs.test.ts b/webapp/channels/src/selectors/lhs.test.ts index d5864b5161a..e5f5d770f4a 100644 --- a/webapp/channels/src/selectors/lhs.test.ts +++ b/webapp/channels/src/selectors/lhs.test.ts @@ -4,6 +4,7 @@ import * as PreferencesSelectors from 'mattermost-redux/selectors/entities/preferences'; import type {GlobalState} from 'types/store'; +import {LhsPage} from 'types/store/lhs'; import * as Lhs from './lhs'; @@ -26,6 +27,41 @@ describe('Selectors.Lhs', () => { state = {}; }); + describe('getIsGlobalThreadsView', () => { + it('returns true when currentStaticPageId is the Threads page', () => { + state = { + views: { + lhs: { + currentStaticPageId: LhsPage.Threads, + }, + }, + }; + expect(Lhs.getIsGlobalThreadsView(state as GlobalState)).toBe(true); + }); + + it('returns false when currentStaticPageId is a different static page', () => { + state = { + views: { + lhs: { + currentStaticPageId: LhsPage.Drafts, + }, + }, + }; + expect(Lhs.getIsGlobalThreadsView(state as GlobalState)).toBe(false); + }); + + it('returns false when no static page is active (channel view)', () => { + state = { + views: { + lhs: { + currentStaticPageId: '', + }, + }, + }; + expect(Lhs.getIsGlobalThreadsView(state as GlobalState)).toBe(false); + }); + }); + describe('should return the open state of the sidebar menu', () => { [true, false].forEach((expected) => { it(`when open is ${expected}`, () => { diff --git a/webapp/channels/src/selectors/lhs.ts b/webapp/channels/src/selectors/lhs.ts index 479abd9561c..7c202185581 100644 --- a/webapp/channels/src/selectors/lhs.ts +++ b/webapp/channels/src/selectors/lhs.ts @@ -11,6 +11,7 @@ import {makeGetDraftsCount} from 'selectors/drafts'; import type {SidebarSize} from 'components/resizable_sidebar/constants'; import type {GlobalState} from 'types/store'; +import {LhsPage} from 'types/store/lhs'; import type {StaticPage} from 'types/store/lhs'; export function getIsLhsOpen(state: GlobalState): boolean { @@ -25,6 +26,10 @@ export function getCurrentStaticPageId(state: GlobalState): string { return state.views.lhs.currentStaticPageId; } +export function getIsGlobalThreadsView(state: GlobalState): boolean { + return state.views.lhs.currentStaticPageId === LhsPage.Threads; +} + export const getDraftsCount = makeGetDraftsCount(); export const getVisibleStaticPages = createSelector(