Refactor IntlProvider in GUI to support integration with www #3431

chrisgarrity · 2018-10-19T14:00:17Z

Resolves

Required for scratchfoundation/scratch-www#2172

Proposed Changes

This is taking advantage of the fact that react-intl can support nested IntlProviders. GUI will use it’s own ConnectedIntlProvider, that maintains the translations in redux, while www will continue to have it’s own IntlProvider that is initialized when the page loads.

Moving the IntlProvider into GUI means that nothing in the playground can use injectIntl. As a result TitledHOC could not use injectIntl, and the localization of the default project name needed to be moved into the component.

The Localization HOC is added, it takes an optional onSetLanguage handler, so www can pass a function that will set the cookie that www needs.

Reason for Changes

Allow localization of GUI when embedded in www

Test Coverage

Current tests run: these changes should not have any impact on stand-alone GUI.

Manual Testing:

Can switch languages
Can load in a different language (e.g. use ?locale=fr on the URL)
Can change the project title
The default project title is translated into other languages

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

Chrome
Firefox
Safari

Windows

Chrome
Firefox
Edge

Chromebook

Chrome

iPad

Safari

Android Tablet

Chrome

rschamp

I think in general this is good, there is just a question of what to do about the defaultProjectTitle message.

src/lib/localization-hoc.jsx

src/reducers/project-title.js

This is taking advantage of the fact that react-intl can support nested IntlProviders. GUI will use it’s own ConnectedIntlProvider, that maintains the translations in redux, while www will continue to have it’s own IntlProvider that is initialized when the page loads. Moving the IntlProvider into GUI meant that nothing in the playground can use injectIntl, so the default project name needed to move into the component instead of being in the reducer. The Localization HOC is added, it takes an optional onSetLanguage handler, so www can pass a function that will set the cookie that www needs.

src/lib/shared-messages.js

        id: 'gui.sharedMessages.pop'
+    },
+    defaultProjectTitle: {
+        id: 'gui.gui.defaultProjectTitle',


benjiwheeler

This looks great, I just think we should hardcode the default English title ('Scratch Project') back in titled-hoc, and take sharedMessages.defaultProjectTitle out of the project-title-input's value prop.

src/components/menu-bar/project-title-input.jsx

                tabIndex="0"
                type="text"
-                value={this.props.projectTitle}
+                value={this.props.projectTitle ?


src/lib/localization-hoc.jsx

+ */
+const LocalizationHOC = function (WrappedComponent) {
+    class LocalizationWrapper extends React.Component {
+        componentDidUpdate (prevProps) {


src/lib/titled-hoc.jsx

            ]);
            this.state = {
-                projectTitle: this.props.intl.formatMessage(defaultProjectTitleMessages.defaultProjectTitle)
+                projectTitle: ''


src/reducers/project-title.js

benjiwheeler

Looks great, thanks!

src/containers/gui.jsx


 import GUIComponent from '../components/gui/gui.jsx';

+const messages = defineMessages({


src/lib/localization-hoc.jsx

+ */
+const LocalizationHOC = function (WrappedComponent) {
+    class LocalizationWrapper extends React.Component {
+        componentDidUpdate (prevProps) {


Addressed in changes

chrisgarrity added pr - needs review localization labels Oct 19, 2018

chrisgarrity added this to the October 2018 milestone Oct 19, 2018

chrisgarrity assigned rschamp Oct 19, 2018

chrisgarrity requested a review from rschamp October 19, 2018 14:00

chrisgarrity mentioned this pull request Oct 19, 2018

Integrate www-gui localization scratchfoundation/scratch-www#2213

Merged

3 tasks

rschamp previously requested changes Oct 19, 2018

View reviewed changes

src/lib/localization-hoc.jsx Show resolved Hide resolved

src/reducers/project-title.js Show resolved Hide resolved

rschamp assigned chrisgarrity and benjiwheeler and unassigned rschamp Oct 19, 2018

rschamp added pr - changes requested needs qa and removed pr - needs review labels Oct 19, 2018

chrisgarrity force-pushed the issue/www2172-integrate-intl branch from c6c82f3 to ce55e7e Compare October 19, 2018 20:54

chrisgarrity force-pushed the issue/www2172-integrate-intl branch from ce55e7e to 8136ac4 Compare October 19, 2018 20:57

move default project title into shared-messages

b8aa33a

chrisgarrity commented Oct 19, 2018

View reviewed changes

src/lib/shared-messages.js Outdated

id: 'gui.sharedMessages.pop'

},

defaultProjectTitle: {

id: 'gui.gui.defaultProjectTitle',

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

chrisgarrity added pr - needs review and removed pr - changes requested labels Oct 19, 2018

benjiwheeler suggested changes Oct 19, 2018

View reviewed changes

benjiwheeler added pr - changes requested and removed pr - needs review labels Oct 19, 2018

benjiwheeler removed their assignment Oct 19, 2018

chrisgarrity mentioned this pull request Oct 22, 2018

Refactor locale handling #3447

Open

move default project name handling fully into gui

2b28d0f

chrisgarrity assigned benjiwheeler and unassigned chrisgarrity Oct 22, 2018

chrisgarrity added pr - needs review and removed pr - changes requested labels Oct 22, 2018

benjiwheeler approved these changes Oct 22, 2018

View reviewed changes

benjiwheeler assigned chrisgarrity Oct 22, 2018

benjiwheeler added pr - good to merge and removed pr - needs review labels Oct 22, 2018

benjiwheeler removed their assignment Oct 22, 2018

chrisgarrity merged commit 92d51a5 into scratchfoundation:develop Oct 22, 2018

chrisgarrity deleted the issue/www2172-integrate-intl branch October 22, 2018 15:59

This was referenced Oct 24, 2018

Language is not handled correctly when embedded in www #3199

Closed

Restore IntlProvider for Internet Explorer #3532

Merged


		import GUIComponent from '../components/gui/gui.jsx';

		const messages = defineMessages({

Refactor IntlProvider in GUI to support integration with www #3431

Refactor IntlProvider in GUI to support integration with www #3431

Uh oh!

Conversation

chrisgarrity commented Oct 19, 2018

Resolves

Proposed Changes

Reason for Changes

Test Coverage

Browser Coverage

Uh oh!

rschamp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

benjiwheeler left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

Uh oh!

benjiwheeler left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants