feat(i18n): improve texts sizes #1204
Conversation
|
Size stats
|
|
Accessibility report ℹ️ You can run this locally by executing |
|
Deploy preview for mistica-web ready! ✅ Preview Built with commit 45c617e. |
src/callout.tsx
Outdated
| const { | ||
| texts, | ||
| i18n: {locale}, | ||
| } = useTheme(); |
There was a problem hiding this comment.
As you can see we now require to get the locale and import a translate function to get the translation.
Also, the locale must be converted to language inside the translate function.
This can be improved.
useTheme could provide the translate function and the language can be precalculated in the context creation, so no extra imports (apart of the tokens) would be needed.
Unless you think a better approach, I will make this change
There was a problem hiding this comment.
choose one
const {t} = useTheme(); // I prefer this one. `t` is commonly used for translations
const {translate} = useTheme();
const {i18n: {t}} = useTheme();
const {i18n: {translate}} = useTheme();
There was a problem hiding this comment.
I like const {translate} = useTheme(); but t is ok too
You could have a different hook just for translations too const translate = useTranslate()
There was a problem hiding this comment.
I prefer to keep it in the same useTheme hook. This way we avoid an additional import and the useTheme is required anyway to get the text overrides
There was a problem hiding this comment.
Using {t} sounds good to me
src/callout.tsx
Outdated
| closeButtonLabel || | ||
| texts.closeButtonLabel || | ||
| translate(closeButtonLabelText, locale) |
There was a problem hiding this comment.
Note that I'm using || instead of ??. I think we can consider empty strings as missing
src/callout.tsx
Outdated
| aria-label={ | ||
| closeButtonLabel || | ||
| texts.closeButtonLabel || | ||
| translate(closeButtonLabelText, locale) |
There was a problem hiding this comment.
You will see this pattern a lot in following files:
texts.patata || translate(patata)I'm unable to think way to avoid having to write this in each token usage (unless we disable text customization)
There was a problem hiding this comment.
Maybe we could have some kind of utility function that does this and use it everywhere?
const getTranslation = (token) => texts[token] || translate(token, locale)
In this way, we avoid having to write the || logic everywhere
There was a problem hiding this comment.
that wouldn't work.
Imagine you want to translate the text defined by "cancel"
// from the provider's context
texts: {
cancel: 'cancelar`,
...
}
// from text-tokens.tsx
export const cancel = {es: 'cancelar', en: 'cancel'...}.Then...
const getTranslation = (token) => { // ----> is this token value the "cancel" string?
if (texts.token) { // ---> this would be `texts["cancel"]`. That's fine
return texts.token;
}
return translate(???, locale); // ---> the translate function accepts as parameter a Token type,
// which is an object like {es: 'cancelar', en: 'cancel'...}.
// We can't use the "cancel" string here as parameter
}There was a problem hiding this comment.
Hmm, I see :(
I think we can proceed with what we have now, and if we come up with some ideas in the future, we can iterate on this
# [15.19.0](v15.18.0...v15.19.0) (2024-09-03) ### Bug Fixes * **Buttons:** avoid warnings related to change in order of react hooks ([#1229](#1229)) ([2dbc411](2dbc411)) * **i18n): Revert "feat(i18n:** improve texts sizes" because it is breaking ([#1226](#1226)) ([79eb4a4](79eb4a4)) * **Logo:** fix webpackChunkName magic comments ([#1214](#1214)) ([3d1f098](3d1f098)) * **Vivinho char:** vivinho char in headings being read as a separated heading ([#1209](#1209)) ([f0f5fb0](f0f5fb0)) ### Features * **Buttons:** refactor code and fix spacing bug in loading buttonLink ([#1212](#1212)) ([640e429](640e429)) * **i18n:** improve texts sizes ([#1204](#1204)) ([0345e7c](0345e7c)) * **Logo:** Refactor logo to improve bundle size and loading times ([#1210](#1210)) ([15b77cb](15b77cb)) * **NavigationBar, FunnelNavigationBar, MainNavigationBar:** add alternative variant ([#1200](#1200)) ([eef87ec](eef87ec))
|
🎉 This PR is included in version 15.19.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
text-tokens.tsx. Once the text tokens defined by design are ready we could autogenerate this fileTokenis an object like:{es: 'Patata', en: 'Potato', de: 'Kartofel', ...}