-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MM-12740 Move translations into redux store and remove LocalizationStore #1896
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments, but nothing blocking. Very nice review: really easy to follow the changes :)
components/root/root.jsx
Outdated
$(window).unbind('storage'); | ||
} | ||
|
||
render() { | ||
if (this.state.translations == null || this.state.configLoaded === false) { | ||
if (this.state.configLoaded === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: !this.state.configLoaded
, or are we intentionally ignoring falsy but !== false
values here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Not intentional.
@@ -1382,7 +1382,7 @@ | |||
"analytics.team.totalUsers": "Total Active Users", | |||
"announcement_bar.error.email_verification_required": "Check your email at {email} to verify the address. Cannot find the email?", | |||
"announcement_bar.error.license_expired": "Enterprise license is expired and some features may be disabled. [Please renew](!{link}).", | |||
"announcement_bar.error.license_expiring": "Enterprise license expires on {date}. [Prease renew](!{link}).", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch!
return team && !team.delete_at > 0 && team.display_name != null; | ||
}). | ||
sort(sortTeamsByDisplayName); | ||
return teams.filter((team) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could just use a expressions here:
return teams.filter((team) => (
team && team.delete_at > 0 && team.display_name != null
)).sort((a, b) => (
compareTeamsByDisplayName(locale, a, b)
));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0/5 on how we do those. I'm just in the habit of always using a function body when it stretches over multiple lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ore (mattermost#1896) * MM-12581 Move translations into redux store and remove LocalizationStore * Switch i18n file mocking to not use identity-obj-proxy * Address feedback
Saying things like "It's a fairly simple store anyway" seems to cause me a lot of problems.
This PR does a few major things:
Ticket Link
https://mattermost.atlassian.net/browse/MM-12740
Checklist