-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: wrap ErrorPage with IntlProvider and pass messages #353
Conversation
092951e
to
0cb10d9
Compare
@@ -33,7 +32,7 @@ subscribe(APP_READY, () => { | |||
}); | |||
|
|||
subscribe(APP_INIT_ERROR, (error) => { | |||
ReactDOM.render(<IntlProvider><ErrorPage message={error.message} /></IntlProvider>, document.getElementById('root')); | |||
ReactDOM.render(<ErrorPage message={error.message} />, document.getElementById('root')); |
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.
ErrorPage
includes IntlProvider
already now.
Codecov Report
@@ Coverage Diff @@
## master #353 +/- ##
==========================================
+ Coverage 81.24% 81.39% +0.14%
==========================================
Files 38 38
Lines 917 919 +2
Branches 170 170
==========================================
+ Hits 745 748 +3
+ Misses 160 159 -1
Partials 12 12
Continue to review full report at Codecov.
|
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.
I'm not a frontend expert, but this looks great and we definitely need this downstream!
Description:
The
ErrorPage
relies on translated messages via react-intl'sFormattedMessage
component. However, it currently relies on consumers (i.e., MFEs) to wrapErrorPage
with anIntlProvider
and pass alocale
andmessages
prop. Instead, this PR proposes adding anIntlProvider
toErrorPage
, pre-configuring itslocale
andmessages
props from the existing configuration in@edx/frontend-platform
via its initialization.Merge checklist:
frontend-platform
. This can be done by runningnpm start
and opening http://localhost:8080.module.config.js
file infrontend-build
.fix
,feat
) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.Post merge: