Skip to content

Commit

Permalink
MM-61733: Fix system console theme stability (mattermost#29257)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebroseland authored Nov 14, 2024
1 parent fb0582c commit 64e814c
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 12 deletions.
12 changes: 12 additions & 0 deletions webapp/channels/src/actions/websocket_actions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,10 @@ function handleSidebarCategoryCreated(msg) {
return (doDispatch, doGetState) => {
const state = doGetState();

if (!msg.broadcast.team_id) {
return;
}

if (msg.broadcast.team_id !== getCurrentTeamId(state)) {
// The new category will be loaded when we switch teams.
return;
Expand All @@ -1518,6 +1522,10 @@ function handleSidebarCategoryUpdated(msg) {
return (doDispatch, doGetState) => {
const state = doGetState();

if (!msg.broadcast.team_id) {
return;
}

if (msg.broadcast.team_id !== getCurrentTeamId(state)) {
// The updated categories will be loaded when we switch teams.
return;
Expand All @@ -1533,6 +1541,10 @@ function handleSidebarCategoryDeleted(msg) {
return (doDispatch, doGetState) => {
const state = doGetState();

if (!msg.broadcast.team_id) {
return;
}

if (msg.broadcast.team_id !== getCurrentTeamId(state)) {
// The category will be removed when we switch teams.
return;
Expand Down
59 changes: 50 additions & 9 deletions webapp/channels/src/components/root/root.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import * as GlobalActions from 'actions/global_actions';
import testConfigureStore from 'packages/mattermost-redux/test/test_store';
import {renderWithContext, waitFor} from 'tests/react_testing_utils';
import {StoragePrefixes} from 'utils/constants';
import * as Utils from 'utils/utils';

import {handleLoginLogoutSignal, redirectToOnboardingOrDefaultTeam} from './actions';
import type {Props} from './root';
Expand Down Expand Up @@ -49,14 +50,9 @@ jest.mock('components/team_sidebar', () => () => <div/>);
jest.mock('components/mobile_view_watcher', () => () => <div/>);
jest.mock('./performance_reporter_controller', () => () => <div/>);

jest.mock('utils/utils', () => {
const original = jest.requireActual('utils/utils');

return {
...original,
applyTheme: jest.fn(),
};
});
jest.mock('utils/utils', () => ({
applyTheme: jest.fn(),
}));

jest.mock('actions/global_actions', () => ({
redirectUserToDefaultTeam: jest.fn(),
Expand All @@ -74,7 +70,7 @@ describe('components/Root', () => {
const store = testConfigureStore();

const baseProps: Props = {
theme: {} as Theme,
theme: {sidebarBg: 'color'} as Theme,
isConfigLoaded: true,
telemetryEnabled: true,
noAccounts: false,
Expand Down Expand Up @@ -372,6 +368,51 @@ describe('components/Root', () => {
});
});
});

describe('applyTheme', () => {
test('should apply theme initially and on change', async () => {
const props = {
...baseProps,
};

const {rerender} = renderWithContext(<Root {...props}/>);

await waitFor(() => {
expect(Utils.applyTheme).toHaveBeenCalledWith(props.theme);
});

const props2 = {
...props,
theme: {sidebarBg: 'color2'} as Theme,
};

rerender(<Root {...props2}/>);

expect(Utils.applyTheme).toHaveBeenCalledWith(props2.theme);
});

test('should not apply theme in system console', async () => {
const props = {
...baseProps,
...{
location: {
pathname: '/admin_console',
},
} as RouteComponentProps,
};

const {rerender} = renderWithContext(<Root {...props}/>);

const props2 = {
...props,
theme: {sidebarBg: 'color2'} as Theme,
};

rerender(<Root {...props2}/>);

expect(Utils.applyTheme).not.toHaveBeenCalled();
});
});
});

describe('doesRouteBelongToTeamControllerRoutes', () => {
Expand Down
17 changes: 14 additions & 3 deletions webapp/channels/src/components/root/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {setSystemEmojis} from 'mattermost-redux/actions/emojis';
import {setUrl} from 'mattermost-redux/actions/general';
import {Client4} from 'mattermost-redux/client';
import {rudderAnalytics, RudderTelemetryHandler} from 'mattermost-redux/client/rudder';
import {Preferences} from 'mattermost-redux/constants';

import {measurePageLoadTelemetry, temporarilySetPageLoadContext, trackEvent, trackSelectorMetrics} from 'actions/telemetry_actions.jsx';
import BrowserStore from 'stores/browser_store';
Expand Down Expand Up @@ -191,7 +192,7 @@ export default class Root extends React.PureComponent<Props, State> {

this.showLandingPageIfNecessary();

applyTheme(this.props.theme);
this.applyTheme();
};

private showLandingPageIfNecessary = () => {
Expand Down Expand Up @@ -255,9 +256,19 @@ export default class Root extends React.PureComponent<Props, State> {
BrowserStore.setLandingPageSeen(true);
};

applyTheme() {
// don't apply theme when in system console; system console hardcoded to THEMES.denim
// AdminConsole will apply denim on mount re-apply user theme on unmount
if (this.props.location.pathname.startsWith('/admin_console')) {
return;
}

applyTheme(this.props.theme);
}

componentDidUpdate(prevProps: Props, prevState: State) {
if (!deepEqual(prevProps.theme, this.props.theme)) {
applyTheme(this.props.theme);
this.applyTheme();
}

if (this.props.location.pathname === '/') {
Expand Down Expand Up @@ -452,7 +463,7 @@ export default class Root extends React.PureComponent<Props, State> {
>
<Switch>
<LoggedInRoute
theme={this.props.theme}
theme={Preferences.THEMES.denim}
path={'/admin_console'}
component={AdminConsole}
/>
Expand Down

0 comments on commit 64e814c

Please sign in to comment.