-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Introduce i18nContext #31347
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
base: develop
Are you sure you want to change the base?
Introduce i18nContext #31347
Conversation
* Adds a context that holds the module i1n API * Switches shared components to use that instead of importing it directly * Adds the context to MatrixChat and BaseDalog so it should be available most places in EW This is a relatively small PR but does change the way the shared components do i18n so just doing this one by itself (it stands by itself anyway). This will allow shared components to use i18n when used in modules.
Then it should continue to get picked up by the script This seems a bit flaky and ew but I'm not sure I want to get into changing this in this PR.
florianduros
left a comment
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.
We can avoid to declare in each stories the i18n provider. Also should we this new i18n api instead of using:
element-web/packages/shared-components/.storybook/preview.tsx
Lines 61 to 63 in 3112a35
| async function languageLoader(context: StoryContext<ReactRenderer, StrictArgs>): Promise<void> { | |
| await setLanguage(context.globals.language); | |
| } |
| }); | ||
| return <AudioPlayerView vm={vm} />; | ||
| return ( | ||
| <I18nContext.Provider value={new I18nApi()}> |
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.
This should be a decorator in .storybook/preview.ts like for the TooltipProvider. so every stories will be wrapped inside this provider.
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.
D'oh, I thought there ought to be a better way of doing that - should have just asked. Thanks.
Not sure what you meant by this? Also yeah, we could certainly add more stuff like setLanguage (and probably should): I can add it it to this PR but thought it might be better to keep it minimal to what we need right now. |
| <I18nContext.Provider value={new I18nApi()}> | ||
| <TooltipProvider> | ||
| <Story /> | ||
| </TooltipProvider> | ||
| </I18nContext.Provider> |
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.
Can you move it to its own storybook decorator? So we each context has its own decorator
This is a relatively small PR (or started as one) but does change the way the shared components do i18n so just doing this one by itself (it stands by itself anyway).
This will allow shared components to use i18n when used in modules.
Needs element-hq/element-modules#137 and a release of the module API for type check to pass.
Checklist
public/exportedsymbols have accurate TSDoc documentation.