Implement new UI for switching languages #12544

jsnajdr · 2017-03-27T10:49:42Z

This is a PR for my trial project described in my post on the i18n P2 blog (internal ref: pxLjZ-3EJ-p2).

The main part of the effort is implementing a new language picker component (internal ref: p4TIVU-5np-p2).

A big task with many subtasks:

A related task that will probably be an independent PR: avoid reloading the whole page after a language is switched. Just trigger a React rerender with the new language. (Moved to #13712)

Does the localize HOC from i18n-calypso support this? Will the value of the injected translate property change?
Will all components properly rerender when translate props changes? Or will it be blocked by some shouldComponentUpdate implementation? Or a wrong/missing componentWillReceiveProps?
Does something that's not managed by React need to change on language switch? Like CSS styles or other assets?
Decide what directory should the new components be in. Is it client/components, or client/blocks or client/components/form? Result: It should be client/component - it's a "dumb" component that only renders what it's passed in props. To be eligible to be a block, it would need to connect to the app state by itself.
Consider removing the current LanguageSelector component and replace it completely with the LanguageChooser, maybe even reusing the name.

The LanguageChooser uses the lib/user-settings module that uses a legacy, event-based approach to data. Converting this module to Redux would be great.

Create a Redux state slice for user settings and provide appropriate action creators and selectors as the new API (happens in a separate PR: Reduxify the user-settings module (/me/settings endpoint) #12819)
Convert the existing lib/user-settings into a thin wrapper over the Redux API
Migrate all usages of lib/user-settings
Remove the legacy lib/user-settings module

akirk · 2017-03-27T12:45:27Z

I have tried it out on my machine: great progress! (Please tell us when you're ready for UI feedback :) ) Does the list represent the priority sequence in which you plan to implement the other points?

Re the Content-Language header: I would suggest not to do a work-around for missing navigator.languages support (for now; btw, it's available in Opera as it's Chrome based). Ultimately we can integrate the Content-Language reporting into locale-guesser to avoid an additional request round trip.

It would be great if you could provide screenshots of the current implementation state, it's both great for documenting the progress as well as it allows others to quickly peak at the current stage.

jsnajdr · 2017-03-27T20:03:29Z

I have tried it out on my machine: great progress! (Please tell us when you're ready for UI feedback :) ) Does the list represent the priority sequence in which you plan to implement the other points?

The list is in a rather random order - it's a memory dump where I wanted to write things down before I forget them. I believe that all or almost all of them need to be implemented before the feature is shippable. Is there anything that you think is likely to spark some longer discussion and that should be implemented sooner rather than later?

It would be great if you could provide screenshots of the current implementation state, it's both great for documenting the progress as well as it allows others to quickly peak at the current stage.

Here is a screenshot of the state as of today:

It shows a part of the my/account form with the LanguageChooser and also the SitesDropdown for comparison. On the right, there is the LanguageChooserModal dialog.

A design question for @folletto: when I see the LanguageChooser and SitesDropdown next to each other, I feel they don't fit together very well. The "English" and "Change" labels are big and bold, while the text in the SitesDropdown looks more subtle. I tried to change the font weight and sizes slightly, and the result is in the bottom left. I like it more. What do you think?

folletto · 2017-03-28T11:33:29Z

I tried to change the font weight and sizes slightly, and the result is in the bottom left. I like it more. What do you think?

A step back for the rationale: it was bold to match the button styling, not the site dropdown styling.

I agree there's a mismatch there tho, as the site dropdown doesn't adjust weight to align with buttons, so it's probably something to be reviewed more broadly as there's an existing incoherence there.

Given we're "in between", I'm ok with either option. If you feel the normal weight one is better, go for it. :)

ehg · 2017-03-28T14:34:25Z

client/components/language-chooser/index.jsx

+		} ),
+	}
+
+	constructor( props ) {


Looks like you could use ESNext property initialisers instead of this constructor.

ehg · 2017-03-28T14:35:07Z

client/components/language-chooser/language-chooser-modal.jsx

@@ -0,0 +1,127 @@
+import React, { PureComponent } from 'react';


Missing an External dependencies docblock

a8ci18n · 2017-03-28T21:13:08Z

client/components/language-chooser/language-chooser-modal.jsx

+			},
+			{
+				action: 'confirm',
+				label: translate( 'Select Language' ),


Hi! I've found a possible matching string that has already been translated 60 times:
translate( 'Select a language below' ) ES Score: 10
See 1 additional suggestion in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

jsnajdr · 2017-03-28T21:29:22Z

Today's update:

added a language button into the sidebar footer
cleaned up the language chooser modal:
- highlight the selected language in the dialog
- save and close the dialog only after clicking the confirmation button

New problem: in order to be able to react to changes in the user settings, and to do that without using legacy components like the observe mixin, we'll need to move the user-settings module functionality (fetching, getting, updating, saving) to a Redux state subtree with appropriate actions, reducers and selectors. Quite a big task.

As a rather ugly temporary workaround, I now subscribe to userSettings' change event in LanguageButton's componentDidMount/componentWillUnmount.

jsnajdr · 2017-03-30T17:55:10Z

Added a list of suggested languages:

It's based on browser preferences, i.e., on navigator.languages. If the browser doesn't support this feature, the suggested list is not displayed at all.

What remains to be done on this feature:

make it responsive - in a narrow viewport, the list overflows and is not pretty.
make the selection highlight box the same size in both the language list and the selected language list. Now the heights are different, for some strange reasons.
test on Safari (Calypso in devel mode often crashes it)
test on IE

@folletto What do you think are best options for making the suggested language list responsive? Show limited number of suggestions on one row and hide the ones that would overflow? Convert the list into a popup menu when the viewport width is small? (SectionNav does exactly this with tabs, maybe we can reuse it)

deBhal · 2017-04-01T21:32:50Z

client/components/language-chooser/index.jsx

+					</div>
+				</div>
+				<LanguageChooserModal
+					isVisible={ this.state.open }


Does the LanguageChooserModal do anything if it's not visible or own the visibility in some sense? If not, we might take the isVisible out of it and check it here to simplify the component and avoid creating it at all when it's not visible.

The LanguageChooserModal internally uses the <Dialog> component which owns the visibility state. It's supposed to be used in a 'declarative' way - just unconditionally render a <Dialog isVisible={...}/> and it will figure out whether to render something or not.

Before this feature is ready to merge, I'll need to do some performance tuning on this: to avoid creating the whole dialog contents in cases where it's not going to be rendered anyway.

jsnajdr · 2017-04-03T07:52:28Z

Latest update: while the user settings are being fetched, all the language chooser buttons show a placeholder with the 'loading' status:

My next steps will be breaking this big PR into smaller ones that can be independently reviewed and merged.

jsnajdr · 2017-04-03T09:25:25Z

Rebased this PR to latest master, where #12714 just got merged.

Removed usages of valueLink and checkedLink from client/me/account/main.jsx and replaced them with value/checked/onChange properties. Also, stopped using valueLink method from me/form-base and the LinkedStateMixin. Replaced them with methods on the Account class that directly work with the userSettings module and the component state. This will come handy when Reduxifying the component - all the method calls will be straithforwardly replaced with selectors and action dispatches. It also comes handy for PR #12544 that introduces a new LanguageChooser component. This component won't have to support the dual API with both value/onChange and valueLink, but will need to support just one. After this PR, doing the same change in other client/me subdirectories should follow.

jsnajdr · 2017-04-03T15:22:39Z

Current status:

rebased the whole PR on top of Removed usages of valueLink from client/me/account #12729 (removal of valueLink usages) and changed the API of LanguageChooser to no longer support valueLink
used the LanguageChooser on the Site Settings form

@akirk @folletto @ehg I guess I'm ready for a design review and code review of the new language picker. There are currently 5 commits in this branch that I indend to merge as separate PRs.

The first version of the chooser dialog will have two tabs with "Popular Languages" and "All Languages", there will be no suggested languages and no division by territory. These will be followups after the basic version is accepted and merged.

I'll be now working on Reduxization of the lib/user-settings module. Although it will be quite a big change, it won't affect the language picker code very much.

folletto · 2017-04-03T18:31:13Z

There are currently 5 commits in this branch that I indend to merge as separate PRs.

Ok go ahead for the separate PRs.

In general the design looks good.

I've just a secondary note that the "button" should be entirely clickable, and should highlight similar to the standard buttons on hover, not just "Change". Sorry if it wasn't clear from the mockups.

Removed usages of valueLink and checkedLink from client/me/account/main.jsx and replaced them with value/checked/onChange properties. Also, stopped using valueLink method from me/form-base and the LinkedStateMixin. Replaced them with methods on the Account class that directly work with the userSettings module and the component state. This will come handy when Reduxifying the component - all the method calls will be straithforwardly replaced with selectors and action dispatches. It also comes handy for PR #12544 that introduces a new LanguageChooser component. This component won't have to support the dual API with both value/onChange and valueLink, but will need to support just one. After this PR, doing the same change in other client/me subdirectories should follow.

folletto · 2017-04-07T12:13:45Z

Ok, looks good:

Removed usages of valueLink and checkedLink from client/me/account/main.jsx and replaced them with value/checked/onChange properties. Also, stopped using valueLink method from me/form-base and the LinkedStateMixin. Replaced them with methods on the Account class that directly work with the userSettings module and the component state. This will come handy when Reduxifying the component - all the method calls will be straithforwardly replaced with selectors and action dispatches. It also comes handy for PR #12544 that introduces a new LanguageChooser component. This component won't have to support the dual API with both value/onChange and valueLink, but will need to support just one. After this PR, doing the same change in other client/me subdirectories should follow.

a8ci18n · 2017-04-14T10:49:18Z

client/components/language-picker/modal.jsx

+			},
+			{
+				action: 'confirm',
+				label: translate( 'Select Language' ),


Hi! I've found a possible matching string that has already been translated 60 times:
translate( 'Select a language below' ) ES Score: 10
See 1 additional suggestion in the PR translation status page

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

yoavf · 2017-04-20T08:30:42Z

Suggestion: use the Autonym Font for the language names to prevent ܕܥܒܪܸܝܛ

Added a new LanguagePicker component which can be used as a form element to change the interface or site language. On click, the component opens a LanguagePickerModal dialog with UI suitable for finding and selecting one of 135+ languages.

When dispatching saveUserSettings, optionally add custom onSuccess and onFailure actions. These will be dispatched instead of the default ones (i.e., the initiator action with meta attached).

Created a LanguageButton component that shows the interface language in every sidebar (shown next to the Help button) and allows to change the language on click.

Display list of suggested languages based on 'navigator.languages', i.e., on browser settings. If the 'navigator.languages' feature is missing from the browser, detect that fact and don't show the suggested list in that case.

jsnajdr · 2017-04-20T15:02:40Z

Suggestion: use the Autonym Font for the language names to prevent ܕܥܒܪܸܝܛ

Looks like a great idea, thanks! I'll definitely try the font out. I'm wondering how the font glyph look like design-wise. Also, I'm a bit worried that the project is explicitly marked as unmaintained.

@yoavf do you have any direct experience with the font? Have used it on any project?

jsnajdr · 2017-04-20T15:05:10Z

Update: using the new Redux state/user-settings module to implement the sidebar button. Works very well, except there are some problems reacting to success/error response after saving the new language. Will blog about it.

yoavf · 2017-04-20T15:36:30Z

Also, I'm a bit worried that the project is explicitly marked as unmaintained.
@yoavf do you have any direct experience with the font? Have used it on any project?

Nope, no personal experience. I was thinking it was used on Wikipedia, but I've just checked and this doesn't seem to be the case. Feel free to ignore it then :)

jsnajdr · 2017-05-07T15:08:36Z

Closing this PR as all related changes were moved to separate smaller PRs under the Language Picker Project

jsnajdr added the [Status] In Progress label Mar 27, 2017

jsnajdr self-assigned this Mar 27, 2017

matticbot added the OSS Citizen label Mar 27, 2017

ehg added i18n Translator labels Mar 27, 2017

ehg reviewed Mar 28, 2017

View reviewed changes

jsnajdr force-pushed the add/language-chooser branch from d7815ed to 728ad4e Compare March 28, 2017 21:05

a8ci18n reviewed Mar 28, 2017

View reviewed changes

jsnajdr force-pushed the add/language-chooser branch from 728ad4e to f8bcedb Compare March 30, 2017 17:46

matticbot added the [Size] XL Probably needs to be broken down into multiple smaller issues label Mar 30, 2017

deBhal reviewed Apr 1, 2017

View reviewed changes

jsnajdr mentioned this pull request Apr 3, 2017

Update/normalize language names #12714

Merged

jsnajdr force-pushed the add/language-chooser branch from f8bcedb to aacd47a Compare April 3, 2017 07:33

jsnajdr force-pushed the add/language-chooser branch from aacd47a to 3174adf Compare April 3, 2017 09:22

jsnajdr mentioned this pull request Apr 3, 2017

Removed usages of valueLink from client/me/account #12729

Merged

jsnajdr force-pushed the add/language-chooser branch from 3174adf to 2a60256 Compare April 3, 2017 15:13

jsnajdr force-pushed the add/language-chooser branch from 2a60256 to 6cad6b8 Compare April 5, 2017 06:23

jsnajdr force-pushed the add/language-chooser branch from 6cad6b8 to 474716b Compare April 5, 2017 10:26

jsnajdr mentioned this pull request Apr 14, 2017

Add new LanguagePicker component #13112

Merged

jsnajdr force-pushed the add/language-chooser branch from b597d56 to 94cb718 Compare April 14, 2017 10:44

a8ci18n reviewed Apr 14, 2017

View reviewed changes

jsnajdr force-pushed the add/language-chooser branch 2 times, most recently from 6cdbc0a to 01b5936 Compare April 17, 2017 08:42

jsnajdr added 6 commits April 20, 2017 16:31

Add new LanguagePicker component

30a6950

Added a new LanguagePicker component which can be used as a form element to change the interface or site language. On click, the component opens a LanguagePickerModal dialog with UI suitable for finding and selecting one of 135+ languages.

Add custom onSuccess and onFailure actions to saveUserSettings

818e24f

When dispatching saveUserSettings, optionally add custom onSuccess and onFailure actions. These will be dispatched instead of the default ones (i.e., the initiator action with meta attached).

Use the new LanguagePicker in the client/me/account form

a589ef1

Use the new LanguagePicker in client/my-sites/site-settings

a3b52cd

Added a language switching button to the sidebar footer

44190e7

Created a LanguageButton component that shows the interface language in every sidebar (shown next to the Help button) and allows to change the language on click.

Add suggested languages to LanguagePickerModal

5ab51cb

Display list of suggested languages based on 'navigator.languages', i.e., on browser settings. If the 'navigator.languages' feature is missing from the browser, detect that fact and don't show the suggested list in that case.

jsnajdr force-pushed the add/language-chooser branch from 01b5936 to 5ab51cb Compare April 20, 2017 14:40

This was referenced Apr 25, 2017

Use the new LanguagePicker in the account and site settings forms #13381

Merged

Add a language switching button to sidebar footer #13454

Closed

jsnajdr closed this May 7, 2017

jsnajdr deleted the add/language-chooser branch May 7, 2017 15:08

matticbot removed the [Status] In Progress label May 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new UI for switching languages #12544

Implement new UI for switching languages #12544

jsnajdr commented Mar 27, 2017 •

edited

Loading

akirk commented Mar 27, 2017 •

edited

Loading

jsnajdr commented Mar 27, 2017

folletto commented Mar 28, 2017

ehg Mar 28, 2017

jsnajdr Mar 28, 2017

ehg Mar 28, 2017

jsnajdr Mar 28, 2017

a8ci18n Mar 28, 2017 •

edited

Loading

jsnajdr commented Mar 28, 2017

jsnajdr commented Mar 30, 2017 •

edited

Loading

deBhal Apr 1, 2017

jsnajdr Apr 3, 2017

jsnajdr commented Apr 3, 2017

jsnajdr commented Apr 3, 2017

jsnajdr commented Apr 3, 2017

folletto commented Apr 3, 2017

folletto commented Apr 7, 2017

a8ci18n Apr 14, 2017

yoavf commented Apr 20, 2017

jsnajdr commented Apr 20, 2017

jsnajdr commented Apr 20, 2017

yoavf commented Apr 20, 2017

jsnajdr commented May 7, 2017

		@@ -0,0 +1,127 @@
		import React, { PureComponent } from 'react';

Implement new UI for switching languages #12544

Implement new UI for switching languages #12544

Conversation

jsnajdr commented Mar 27, 2017 • edited Loading

akirk commented Mar 27, 2017 • edited Loading

jsnajdr commented Mar 27, 2017

folletto commented Mar 28, 2017

ehg Mar 28, 2017

Choose a reason for hiding this comment

jsnajdr Mar 28, 2017

Choose a reason for hiding this comment

ehg Mar 28, 2017

Choose a reason for hiding this comment

jsnajdr Mar 28, 2017

Choose a reason for hiding this comment

a8ci18n Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

jsnajdr commented Mar 28, 2017

jsnajdr commented Mar 30, 2017 • edited Loading

deBhal Apr 1, 2017

Choose a reason for hiding this comment

jsnajdr Apr 3, 2017

Choose a reason for hiding this comment

jsnajdr commented Apr 3, 2017

jsnajdr commented Apr 3, 2017

jsnajdr commented Apr 3, 2017

folletto commented Apr 3, 2017

folletto commented Apr 7, 2017

a8ci18n Apr 14, 2017

Choose a reason for hiding this comment

yoavf commented Apr 20, 2017

jsnajdr commented Apr 20, 2017

jsnajdr commented Apr 20, 2017

yoavf commented Apr 20, 2017

jsnajdr commented May 7, 2017

jsnajdr commented Mar 27, 2017 •

edited

Loading

akirk commented Mar 27, 2017 •

edited

Loading

a8ci18n Mar 28, 2017 •

edited

Loading

jsnajdr commented Mar 30, 2017 •

edited

Loading