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

Fix rendering of timezone info inside chats when the selected language is Spanish #4309

Merged
merged 1 commit into from
Jul 29, 2021
+24 −25
Merged

Fix rendering of timezone info inside chats when the selected language is Spanish #4309

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 24 additions & 25 deletions src/pages/home/report/ParticipantLocalTime.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@ class ParticipantLocalTime extends React.Component {
}, 1000));
}

shouldComponentUpdate(nextProps, nextState) {
return nextState.localTime !== this.state.localTime;
}

componentWillUnmount() {
clearInterval(this.timer);
clearInterval(this.readyTimer);
}

getParticipantLocalTime() {
const reportRecipientTimezone = lodashGet(this.props.participant, 'timezone', {});
moment.locale(this.props.preferredLocale);
Copy link
Member

Choose a reason for hiding this comment

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

This will cause issues with moment() usages. it updates the locale to another user's timezone globally. There are many places where we are dependent on the moment() to get the time.

Do you think it's safe to use it this way?

Copy link
Contributor

Choose a reason for hiding this comment

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

@parasharrajat I haven't reviewed this code yet, in the middle of testing. Can you give me an example of a type of regression this could cause? So far in my testing things are working great and haven't noticed issues with displayed times yet.

Copy link
Member

@parasharrajat parasharrajat Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please look at ReimbursementAccountPage L174 & CompanyStep L107. Dates could be +1 or -1 based on timezone etc. If your testing shows fine results then all good.

const reportRecipientDay = moment().tz(reportRecipientTimezone.selected).format('dddd');
const currentUserDay = moment().tz(this.props.currentUserTimezone.selected).format('dddd');

Expand All @@ -52,38 +57,32 @@ class ParticipantLocalTime extends React.Component {
}

render() {
// Moment.format does not return AM or PM values immediately.
// So we have to wait until we are ready before showing the time to the user
const isReportRecipientLocalTimeReady = this.state.localTime.toString().match(/(A|P)M/ig);
const reportRecipientDisplayName = this.props.participant.firstName
|| (Str.isSMSLogin(this.props.participant.login)
? this.props.toLocalPhone(this.props.participant.displayName)
: this.props.participant.displayName);

return (
isReportRecipientLocalTimeReady ? (
<View style={[styles.chatItemComposeSecondaryRow]}>
<View style={[
styles.chatItemComposeSecondaryRowOffset,
styles.flexRow,
styles.alignItemsCenter]}
>
<ExpensiText style={[styles.chatItemComposeSecondaryRowSubText, styles.mr1]}>
{this.props.translate('common.timePrefix')}
</ExpensiText>
<ExpensiText style={[styles.textMicroSupportingBold, styles.mr1]}>
{this.state.localTime}
</ExpensiText>
<ExpensiText style={[styles.chatItemComposeSecondaryRowSubText, styles.mr1]}>
{this.props.translate('common.conjunctionFor')}
</ExpensiText>
<ExpensiText style={[styles.textMicroSupportingBold]}>
{reportRecipientDisplayName}
</ExpensiText>
</View>
<View style={[styles.chatItemComposeSecondaryRow]}>
<View style={[
styles.chatItemComposeSecondaryRowOffset,
styles.flexRow,
styles.alignItemsCenter]}
>
<ExpensiText style={[styles.chatItemComposeSecondaryRowSubText, styles.mr1]}>
{this.props.translate('common.timePrefix')}
</ExpensiText>
<ExpensiText style={[styles.textMicroSupportingBold, styles.mr1]}>
{this.state.localTime}
</ExpensiText>
<ExpensiText style={[styles.chatItemComposeSecondaryRowSubText, styles.mr1]}>
{this.props.translate('common.conjunctionFor')}
</ExpensiText>
<ExpensiText style={[styles.textMicroSupportingBold]}>
{reportRecipientDisplayName}
</ExpensiText>
</View>
)
: <View style={[styles.chatItemComposeSecondaryRow]} />
</View>
);
}
}
Expand Down