Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-14860 Localization for client plugins. #2621

Merged
merged 1 commit into from
Apr 9, 2019
Merged

MM-14860 Localization for client plugins. #2621

merged 1 commit into from
Apr 9, 2019

Conversation

crspeller
Copy link
Member

@crspeller crspeller commented Apr 5, 2019

Summary

This is a functional, ready to merge, proposal on how webapp side plugin translation could work.

The basic process from the plugin side is to use the new PluginRegistery.registerTranslations function to register a callback of the form getTranslations(locale) that delivers translations on demand. The plugin is then responsible for providing translations for English, and every language it supports. How to get those translations and if Mattermost the company gets involved is out of the scope of this PR, but it assumes the translation files will live with the plugins.

One consequence of not of this method is to avoid the translations causing a "screen flash" when they load in, webapp plugin loading has been made synchronous with the rendering of the root component. ie. The root component will not render until all plugins have performed their initialization. This both solves the problem for translations and prevents plugins from popping in later. However it does raise the possibility of long plugin initialization slowing down the initial load of the webapp.

Ticket Link

https://mattermost.atlassian.net/browse/MM-14860

Related Pull Requests

Demo plugin using this: mattermost/mattermost-plugin-demo#25

@crspeller crspeller added the 2: Dev Review Requires review by a core commiter label Apr 5, 2019
@crspeller crspeller marked this pull request as ready for review April 5, 2019 20:03
@jasonblais
Copy link
Contributor

/cc @hanzei FYI

if (this.props.location.pathname === '/' && this.props.noAccounts) {
this.props.history.push('/signup_user_complete');
}

this.setState({configLoaded: true});
initializePlugins().then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just so I understand, is this just cleanup and not directly relevant, or is this so that we now wait until all translations are loaded inside initializePlugin()?

Copy link
Member Author

Choose a reason for hiding this comment

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

We now wait for all plugins to be loaded.

Copy link
Member

@jwilander jwilander left a comment

Choose a reason for hiding this comment

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

Slightly worried about blocking on loading the plugins but I can't think of a good alternative and it does have the nice effect preventing plugins from "popping in"

Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Looks good. I have one small thought, but I don't know if it's anything that requires changes

import {ActionTypes} from 'utils/constants.jsx';

function translations(state = {en}, action) {
function translations(state = {}, action) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the change in when the English translations are added should make a difference, but any idea of we might be using those translations before loadTranslations has been called?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can tell we can't use the translations system before that point, so if there are instances, they don't work anyway.

@crspeller crspeller merged commit 4c0e537 into master Apr 9, 2019
@crspeller crspeller deleted the mm-14860 branch April 9, 2019 18:36
@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Apr 9, 2019
@hanzei hanzei added this to the v5.12.0 milestone Apr 9, 2019
@lindy65 lindy65 added Tests/Not Needed Does not require new release tests and removed 4: Reviews Complete All reviewers have approved the pull request labels Apr 13, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed Does not require new release tests
Projects
None yet
8 participants