-
Notifications
You must be signed in to change notification settings - Fork 13
Drops React-Intl V2-V4 Support #293
Conversation
packages/terra-application/src/application-base/private/intlLoaders.js
Outdated
Show resolved
Hide resolved
Few screenshots of WDIO is having {placeholder-text} instead of translated strings. Please verify if the translations are loading fine dev-site test examples page. |
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.
Nice
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.
Since this is a breaking change, I think it should be merged into a major-version-bump-branch rather than main. If this is merged into main then this change would be released without warning to users.
packages/terra-application/src/application-base/ApplicationBase.jsx
Outdated
Show resolved
Hide resolved
...ra-application-docs/src/terra-dev-site/tool/terra-dev-site/UpgradeGuides.f/v2.0.0.q.tool.mdx
Outdated
Show resolved
Hide resolved
...ra-application-docs/src/terra-dev-site/tool/terra-dev-site/UpgradeGuides.f/v2.0.0.q.tool.mdx
Outdated
Show resolved
Hide resolved
There is an issue with translation strings (can be seen here) that still needs to be resolved. Other than that, this PR looks good to me and will approve once this issue is resolved. |
...ra-application-docs/src/terra-dev-site/tool/terra-dev-site/UpgradeGuides.f/v2.0.0.q.tool.mdx
Outdated
Show resolved
Hide resolved
...ra-application-docs/src/terra-dev-site/tool/terra-dev-site/UpgradeGuides.f/v2.0.0.q.tool.mdx
Outdated
Show resolved
Hide resolved
packages/terra-application/src/application-intl/ApplicationIntlContext.jsx
Show resolved
Hide resolved
packages/terra-application/src/terra-dev-site/app/components.2/ApplicationBase.app.mdx
Outdated
Show resolved
Hide resolved
Resolved all review comments: 6381a86 |
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.
Just too keep note. Terra-dev-site and terra-core-docs packages in terra-application will also need version bump for terra-application
once it is released.
Summary
Changes of this PR were already merged to terra-application-v2 branch by PR #124 and planned to release with terra-application V2 release.
Since terra-application-v2 branch has lot of other unplanned features with it. React-Intl V5 upgrade code has been separated and planned to release with terra-application V2 release.
Changes of this PR includes React-Intl V5 upgrades in Application components and Terra-Base and Terra-I18n Logic has been moved to Application-Base.
here is detailed description of code changes :
Moved i18n logic from terra-i18n and terra-base to be encapsulated in terra-application-base. Locale data loaders that were being generated by terra-aggregate-translations has been hard coded now and logic simplified to utilize a promise api. The biggest change required by react-intl v5 is an update to our polyfills, react-intl also no longer exports locale data. It requires the Intl.PluralRules and Intl.RelativeTimeFormat apis in addition to the one previously required. The polyfill intl that we have been utilizing supports Intl.PluralRules but not Intl.RelativeTimeFormat. formatjs provides polyfills for each of the Intl apis that it requires, but does not supply the locale data for all of the locales that we support. So, I continued to utilize the intl polyfill for the api's that it supplies and the format-JS polyfill for Intl.RelativeTimeFormat.
Difference in File count between this PR and the PR #124 is due to the changes that happened to terra-application after PR124 was merged into terra-application-v2 branches.
Closes #258
Closes #240
Deployment Link
https://terra-applic-.herokuapp.com/
Testing
This changes has been verified in open source repos : Terra-clinical and Terra-Graphs.
( links to open source repos will be added soon !! )
Additional Details
Thank you for contributing to Terra.
@cerner/terra