translate array controls in material renderers #2099
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
LukasBoll
force-pushed
the
translateControlElements
branch
from
1fba52c
to
d0f5c29
Compare
There was a problem hiding this comment.
Hi @LukasBoll , thanks for your pull request :)
I like the general approach of making the array translations available in a typed way.
Please have a look at the inline comments. They are mainly about namings to make things more clear.
Also, testing that translations are properly applied is pretty inconvenient right now. I suggest adding an additional example similar to packages/examples/src/examples/arrays.ts.
The new example could be named Array (i18n)
You can leave out the non-array property and probably do not need the actions to toggle sorting. Instead, the sorting buttons should be shown in the new example to allow checking their aria labels.
| @@ -0,0 +1,40 @@ | |||
| export interface DefaultTranslation { | |||
There was a problem hiding this comment.
Please rename this interface to ArrayDefaultTranslation. The current name suggests that this describes a default translation for any kind of message.
| {key: ArrayTranslationEnum.deleteDialogDecline, default: () => 'NO'} | ||
| ] | ||
|
|
||
| export const arrayTranslations: DefaultTranslation[] = Object.values(arrayDefaultTranslations).map(value=>value); |
There was a problem hiding this comment.
The identity map at the end .map(value=>value) seems superfluous to me. Is this really needed?
packages/core/src/i18n/i18nUtil.ts
Outdated
| @@ -123,3 +131,19 @@ export const deriveLabelForUISchemaElement = (uischema: Labelable<boolean>, t: T | |||
| const i18nKey = typeof i18nKeyPrefix === 'string' ? `${i18nKeyPrefix}.label` : stringifiedLabel; | |||
| return t(i18nKey, stringifiedLabel, { uischema: uischema }); | |||
| } | |||
|
|
|||
| export const translateElements = ( | |||
There was a problem hiding this comment.
This should be renamed to more accurately describe what it does: retrieving translated Array messages.
E.g. the function could be named getArrayTranslations
packages/material-renderers/src/complex/MaterialTableControl.tsx
Outdated
Show resolved
Hide resolved
| @@ -37,13 +37,17 @@ export interface DeleteDialogProps { | |||
| onClose(): void; | |||
| onConfirm(): void; | |||
| onCancel(): void; | |||
| headerText: string, | |||
There was a problem hiding this comment.
I think title describes more precisely what this is used for. Please rename to title
LukasBoll
force-pushed
the
translateControlElements
branch
2 times, most recently
from
0d11318
to
b1c5266
Compare
| } | ||
| } | ||
| }, | ||
| foo:{type:'string'} |
There was a problem hiding this comment.
This is not needed for this example => please delete
| { | ||
| type: 'Control', | ||
| scope: '#/properties/foo' | ||
| }, |
There was a problem hiding this comment.
This is not needed for this example => please delete
|
|
||
| registerExamples([ | ||
| { | ||
| name: 'array (i18n)', |
There was a problem hiding this comment.
The name is used as part of the url fragment. Thus, it's better to avoid spaces and parantheses.
| name: 'array (i18n)', | |
| name: 'array-i18n', |
LukasBoll
force-pushed
the
translateControlElements
branch
from
b1c5266
to
6682da6
Compare
|
Hi @lucas-koehler, |
| } | ||
| }; | ||
| export const translate: Translator = (key: string, defaultMessage: string) => { | ||
| console.log(key); |
There was a problem hiding this comment.
Sorry I forgot this earlier. Please remove this console log before merging :)
There was a problem hiding this comment.
Thank you, I just removed the console log
eclipsesource#1826 Co-authored-by: Lucas Koehler <lkoehler@eclipsesource.com>
LukasBoll
force-pushed
the
translateControlElements
branch
from
6682da6
to
1150122
Compare
translate array controls in material renderers