mirror of
https://github.com/mattermost/mattermost.git
synced 2026-05-27 12:13:29 -04:00
Fix: Global Threads view shows only 1 quick reaction emoji instead of 3 (MM-68681) (#36512)
* 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 <mgdelacroix@users.noreply.github.com>
* 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 <mgdelacroix@users.noreply.github.com>
* 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 <mgdelacroix@users.noreply.github.com>
* 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 <mgdelacroix@users.noreply.github.com>
* 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 <mgdelacroix@users.noreply.github.com>
* Fix types: use correct EmojiCategory value 'people-body' not 'people'
Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com>
* 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 <mgdelacroix@users.noreply.github.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Miguel de la Cruz <mgdelacroix@users.noreply.github.com>
This commit is contained in:
Miguel de la Cruz
GitHub
parent
5cacd26776
commit
02bae8c3a1
5 changed files with 223 additions and 3 deletions
|
|
@ -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';
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
127
webapp/channels/src/components/post/post_options.test.tsx
Normal file
127
webapp/channels/src/components/post/post_options.test.tsx
Normal file
|
|
@ -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(
|
||||
<PostOptions
|
||||
{...baseProps}
|
||||
location={Locations.CENTER}
|
||||
isExpanded={false}
|
||||
hover={true}
|
||||
/>,
|
||||
baseState,
|
||||
);
|
||||
|
||||
expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3);
|
||||
});
|
||||
|
||||
test('RHS_ROOT with isExpanded false (narrow sidebar) shows 1 quick reaction emoji', () => {
|
||||
renderWithContext(
|
||||
<PostOptions
|
||||
{...baseProps}
|
||||
location={Locations.RHS_ROOT}
|
||||
isExpanded={false}
|
||||
hover={true}
|
||||
/>,
|
||||
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(
|
||||
<PostOptions
|
||||
{...baseProps}
|
||||
location={Locations.RHS_ROOT}
|
||||
isExpanded={true}
|
||||
hover={true}
|
||||
/>,
|
||||
baseState,
|
||||
);
|
||||
|
||||
expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3);
|
||||
});
|
||||
|
||||
test('RHS_COMMENT with isExpanded true shows 3 quick reaction emojis', () => {
|
||||
renderWithContext(
|
||||
<PostOptions
|
||||
{...baseProps}
|
||||
location={Locations.RHS_COMMENT}
|
||||
isExpanded={true}
|
||||
hover={true}
|
||||
/>,
|
||||
baseState,
|
||||
);
|
||||
|
||||
expect(screen.getAllByTestId('post-menu__item_emoji')).toHaveLength(3);
|
||||
});
|
||||
});
|
||||
|
|
@ -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}`, () => {
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue