Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove t and localizeMessage from components/activity_log_modal #26987

Merged
merged 2 commits into from
May 13, 2024
Merged

Remove t and localizeMessage from components/activity_log_modal #26987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8893163
Remove t from components/activity_log_modal
hmhealey May 9, 2024
94a9100
Remove localizeMessage from components/activity_log_modal
hmhealey May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions ...els/src/components/activity_log_modal/components/__snapshots__/activity_log.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@ exports[`components/activity_log_modal/ActivityLog should match snapshot 1`] = `
<div
className="report__platform"
>
<i
className="fa fa-linux"
title="Linux Icon"
<DeviceIcon
devicePicture="fa fa-linux"
deviceTitle={
Object {
"defaultMessage": "Linux Icon",
"id": "device_icons.linux",
}
}
/>
<MemoizedFormattedMessage
defaultMessage="Native Desktop App"
Expand Down Expand Up @@ -98,9 +103,14 @@ exports[`components/activity_log_modal/ActivityLog should match snapshot with mo
<div
className="report__platform"
>
<i
className="fa fa-apple"
title="Apple Icon"
<DeviceIcon
devicePicture="fa fa-apple"
deviceTitle={
Object {
"defaultMessage": "Apple Icon",
"id": "device_icons.apple",
}
}
/>
<MemoizedFormattedMessage
defaultMessage="iPhone Native Classic App"
Expand Down
17 changes: 8 additions & 9 deletions webapp/channels/src/components/activity_log_modal/components/activity_log.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {General} from 'mattermost-redux/constants';
import ActivityLog from 'components/activity_log_modal/components/activity_log';

import {TestHelper} from 'utils/test_helper';
import {localizeMessage} from 'utils/utils';

describe('components/activity_log_modal/ActivityLog', () => {
const baseProps = {
Expand Down Expand Up @@ -87,34 +86,34 @@ describe('components/activity_log_modal/ActivityLog', () => {
id='activity_log_modal.iphoneNativeClassicApp'
/>
);
const apple = {devicePicture: 'fa fa-apple', deviceTitle: localizeMessage('device_icons.apple', 'Apple Icon'), devicePlatform: appleText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'apple'}))).toEqual(apple);
const apple = {devicePicture: 'fa fa-apple', devicePlatform: appleText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'apple'}))).toMatchObject(apple);

const androidText = (
<FormattedMessage
defaultMessage='Android Native Classic App'
id='activity_log_modal.androidNativeClassicApp'
/>
);
const android = {devicePicture: 'fa fa-android', deviceTitle: localizeMessage('device_icons.android', 'Android Icon'), devicePlatform: androidText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'android'}))).toEqual(android);
const android = {devicePicture: 'fa fa-android', devicePlatform: androidText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'android'}))).toMatchObject(android);

const appleRNText = (
<FormattedMessage
defaultMessage='iPhone Native App'
id='activity_log_modal.iphoneNativeApp'
/>
);
const appleRN = {devicePicture: 'fa fa-apple', deviceTitle: localizeMessage('device_icons.apple', 'Apple Icon'), devicePlatform: appleRNText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'apple_rn'}))).toEqual(appleRN);
const appleRN = {devicePicture: 'fa fa-apple', devicePlatform: appleRNText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'apple_rn'}))).toMatchObject(appleRN);

const androidRNText = (
<FormattedMessage
defaultMessage='Android Native App'
id='activity_log_modal.androidNativeApp'
/>
);
const androidRN = {devicePicture: 'fa fa-android', deviceTitle: localizeMessage('device_icons.android', 'Android Icon'), devicePlatform: androidRNText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'android_rn'}))).toEqual(androidRN);
const androidRN = {devicePicture: 'fa fa-android', devicePlatform: androidRNText};
expect(mobileSessionInfo(TestHelper.getSessionMock({device_id: 'android_rn'}))).toMatchObject(androidRN);
});
});
97 changes: 64 additions & 33 deletions webapp/channels/src/components/activity_log_modal/components/activity_log.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@
// See LICENSE.txt for license information.

import React from 'react';
import {FormattedDate, FormattedMessage, FormattedTime} from 'react-intl';
import type {MessageDescriptor} from 'react-intl';
import {FormattedDate, FormattedMessage, FormattedTime, defineMessages} from 'react-intl';

import type {Session} from '@mattermost/types/sessions';

import {General} from 'mattermost-redux/constants';

import {getMonthLong, t} from 'utils/i18n';
import {localizeMessage} from 'utils/utils';
import {getMonthLong} from 'utils/i18n';

import DeviceIcon from './device_icon';
import MoreInfo from './more_info';

type Props = {
Expand Down Expand Up @@ -42,8 +43,8 @@ type State = {

type MobileSessionInfo = {
devicePicture?: string;
deviceTitle?: string;
devicePlatform: JSX.Element;
deviceTitle?: MessageDescriptor;
devicePlatform?: JSX.Element;
};

export default class ActivityLog extends React.PureComponent<Props, State> {
Expand All @@ -68,42 +69,52 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
};

mobileSessionInfo = (session: Session): MobileSessionInfo => {
let deviceTypeId;
let deviceTypeMessage;
let devicePlatform;
let devicePicture;
let deviceTitle;

if (session.device_id.includes('apple')) {
devicePicture = 'fa fa-apple';
deviceTitle = localizeMessage('device_icons.apple', 'Apple Icon');
deviceTypeId = t('activity_log_modal.iphoneNativeClassicApp');
deviceTypeMessage = 'iPhone Native Classic App';
deviceTitle = messages.appleIcon;
devicePlatform = (
<FormattedMessage
id='activity_log_modal.iphoneNativeClassicApp'
defaultMessage='iPhone Native Classic App'
/>
);

if (session.device_id.includes(General.PUSH_NOTIFY_APPLE_REACT_NATIVE)) {
deviceTypeId = t('activity_log_modal.iphoneNativeApp');
deviceTypeMessage = 'iPhone Native App';
devicePlatform = (
<FormattedMessage
id='activity_log_modal.iphoneNativeApp'
defaultMessage='iPhone Native App'
/>
);
}
} else if (session.device_id.includes('android')) {
devicePicture = 'fa fa-android';
deviceTitle = localizeMessage('device_icons.android', 'Android Icon');
deviceTypeId = t('activity_log_modal.androidNativeClassicApp');
deviceTypeMessage = 'Android Native Classic App';
deviceTitle = messages.androidIcon;
devicePlatform = (
<FormattedMessage
id='activity_log_modal.androidNativeClassicApp'
defaultMessage='Android Native Classic App'
/>
);

if (session.device_id.includes(General.PUSH_NOTIFY_ANDROID_REACT_NATIVE)) {
deviceTypeId = t('activity_log_modal.androidNativeApp');
deviceTypeMessage = 'Android Native App';
devicePlatform = (
<FormattedMessage
id='activity_log_modal.androidNativeApp'
defaultMessage='Android Native App'
/>
);
}
}

return {
devicePicture,
deviceTitle,
devicePlatform: (
<FormattedMessage
id={deviceTypeId}
defaultMessage={deviceTypeMessage}
/>
),
devicePlatform,
};
};

Expand All @@ -117,7 +128,7 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
const lastAccessTime = new Date(currentSession.last_activity_at);
let devicePlatform = currentSession.props.platform;
let devicePicture: string | undefined = '';
let deviceTitle = '';
let deviceTitle: MessageDescriptor | string = '';

if (this.isMobileSession(currentSession)) {
const sessionInfo = this.mobileSessionInfo(currentSession);
Expand All @@ -127,11 +138,11 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
} else {
if (currentSession.props.platform === 'Windows') {
devicePicture = 'fa fa-windows';
deviceTitle = localizeMessage('device_icons.windows', 'Windows Icon');
deviceTitle = messages.windowsIcon;
} else if (currentSession.props.platform === 'Macintosh' ||
currentSession.props.platform === 'iPhone') {
devicePicture = 'fa fa-apple';
deviceTitle = localizeMessage('device_icons.apple', 'Apple Icon');
deviceTitle = messages.appleIcon;
} else if (currentSession.props.platform === 'Linux') {
if (currentSession.props.os.indexOf('Android') >= 0) {
devicePlatform = (
Expand All @@ -141,14 +152,14 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
/>
);
devicePicture = 'fa fa-android';
deviceTitle = localizeMessage('device_icons.android', 'Android Icon');
deviceTitle = messages.androidIcon;
} else {
devicePicture = 'fa fa-linux';
deviceTitle = localizeMessage('device_icons.linux', 'Linux Icon');
deviceTitle = messages.linuxIcon;
}
} else if (currentSession.props.os.indexOf('Linux') !== -1) {
devicePicture = 'fa fa-linux';
deviceTitle = localizeMessage('device_icons.linux', 'Linux Icon');
deviceTitle = messages.linuxIcon;
}

if (currentSession.props.browser.indexOf('Desktop App') !== -1) {
Expand All @@ -168,10 +179,11 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
>
<div className='activity-log__report'>
<div className='report__platform'>
<i
className={devicePicture}
title={deviceTitle}
/>{devicePlatform}
<DeviceIcon
devicePicture={devicePicture}
deviceTitle={deviceTitle}
/>
{devicePlatform}
</div>
<div className='report__info'>
<div>
Expand Down Expand Up @@ -220,3 +232,22 @@ export default class ActivityLog extends React.PureComponent<Props, State> {
);
}
}

const messages = defineMessages({
androidIcon: {
id: 'device_icons.android',
defaultMessage: 'Android Icon',
},
appleIcon: {
id: 'device_icons.apple',
defaultMessage: 'Apple Icon',
},
linuxIcon: {
id: 'device_icons.linux',
defaultMessage: 'Linux Icon',
},
windowsIcon: {
id: 'device_icons.windows',
defaultMessage: 'Windows Icon',
},
});
31 changes: 31 additions & 0 deletions webapp/channels/src/components/activity_log_modal/components/device_icon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React from 'react';
import type {MessageDescriptor} from 'react-intl';
import {useIntl} from 'react-intl';

import {isMessageDescriptor} from 'utils/i18n';

type Props = {
devicePicture?: string;
deviceTitle: MessageDescriptor | string;
}

export default function DeviceIcon(props: Props) {
const intl = useIntl();

let title;
if (isMessageDescriptor(props.deviceTitle)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0/5 on legibility, but to avoid one import, we can check here typeof props.deviceTitle === 'string'.

And also 0/5 on legibility, but we could do:

const title = typeof props.deviceTitle === 'string' ? props.deviceTitle : intl.formatMessage(props.deviceTitle);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like going with the more descriptive name here than using the typeof even though they could do the same thing. For the ternary, I ran into some cases where that was causing the line to be a bit long.

I was thinking about making a helper like formatToString(i18n: I18n, value: string | MessageDescriptor): string though because I feel like I've written this logic a few times lately. Ideally, I think we'd be consistent about what we pass around, but that seems like it'd be helpful in a bunch of places

title = intl.formatMessage(props.deviceTitle);
} else {
title = props.deviceTitle;
}

return (
<i
className={props.devicePicture}
title={title}
/>
);
}
Loading