Angular Material: Translate Autocomplete Control Renderer#2535
Angular Material: Translate Autocomplete Control Renderer#2535daniel-shuy wants to merge 4 commits intoeclipsesource:masterfrom
Conversation
✅ Deploy Preview for jsonforms-examples ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi @daniel-shuy, You are right, the Angular Material renderer set is the most inconsistent one with the most missing features. Happy to review your contribution(s) ❤️ |
lucas-koehler
left a comment
There was a problem hiding this comment.
Hi @daniel-shuy Thanks for the contribution and extending the tests ❤️ . Allowing translation of the values defintely makes sense!
I have two comments inline. Pease have a look.
packages/angular-material/src/library/controls/autocomplete.renderer.ts
Outdated
Show resolved
Hide resolved
21eaaba to
a7d33df
Compare
There was a problem hiding this comment.
Misclicked before, sorry:
Hi @daniel-shuy ,
thanks for the update!
Unfortunately, it seems that something is broken now:
Looking at the Enums example in the preview https://deploy-preview-2535--jsonforms-examples.netlify.app/angular-material/ selecting a value does not update the value for me. On the master preview this works fine for me.
Can you have a look?
a7d33df to
83360e7
Compare
|
@lucas-koehler sorry, I had some leftover code that depended on |
lucas-koehler
left a comment
There was a problem hiding this comment.
Hi @daniel-shuy , thanks for the update! Generally, the enum controls work again. However, after selecting a value, the untranslated value is shown in the enum control while the dropdown (correctly) shows the translated values.
This should be aligned to also show the translated value in the input. This is for instance already the case in the react-material renderer set.
Find attached an enum i18n example to test this. You can also add this example to the PR.
|
@lucas-koehler Ah, now I remember why I needed I'll also update the tests to assert that the |
|
Hi @daniel-shuy , |
68fd15d to
5302153
Compare
|
@lucas-koehler Great! I've made the changes and updated the tests, it should work now |
lucas-koehler
left a comment
There was a problem hiding this comment.
Thank you for the updates but something is still missing.
When I select an option in the enum i18n example at the preview https://deploy-preview-2535--jsonforms-examples.netlify.app/angular-material/ the control stays empty:
jf-angular-enums.webm
5302153 to
36c55fe
Compare
36c55fe to
995e0ff
Compare
|
@lucas-koehler sorry for the delay, I was away on holiday last week. I've finally managed to fix all the issues. Now I understand why this hasn't been implemented, the Angular Material mat-autocomplete API is so insanely complex 😅 |
lucas-koehler
left a comment
There was a problem hiding this comment.
Hi @daniel-shuy , no worries about the delay - we're not in a rush :D
Thanks for the updates! The renderer now works for me 🚀 I pushed an update for the tests because they should test persisting the enum value - not the enumoption.
Besides that I have a few small suggestions inline. Please have a look :)
|
|
||
| override onChange(ev: any) { | ||
| const eventValue = this.getEventValue(ev); | ||
| const option = Array.from(this.valuesToTranslatedOptions?.values()).find( |
There was a problem hiding this comment.
Array.from(undefined) throws an error, thus have a safe fallback:
| const option = Array.from(this.valuesToTranslatedOptions?.values()).find( | |
| const option = Array.from(this.valuesToTranslatedOptions?.values() ?? []).find( |
|
|
||
| override mapAdditionalProps(_props: StatePropsOfControl & OwnPropsOfEnum) { | ||
| this.valuesToTranslatedOptions = new Map( | ||
| _props.options.map((option) => [option.value, option]) |
There was a problem hiding this comment.
Options may be undefined.
| _props.options.map((option) => [option.value, option]) | |
| (_props.options ?? []).map((option) => [option.value, option]) |
|
|
||
| if (typeof option === 'string') { | ||
| if (!this.valuesToTranslatedOptions) { | ||
| return ''; |
There was a problem hiding this comment.
We should rather show the raw, untranslated value than nothing as long as the translation is not available. Or do you see any issues with that?
| return ''; | |
| return option; // show raw value until translations are ready |
| trackByEnumOptionId(_index: number, option: EnumOption): string { | ||
| return option.value; | ||
| } |
There was a problem hiding this comment.
This method seems to be unused now. Can it be removed?
Background
The company I work for (SICPA Product Security) is considering adopting JSON Forms for a project.
Unfortunately the project is written in Angular, and I noticed that the Angular Material renderer set is missing quite a lot of supported features compared to the React/Vue renderer sets.
As such, I have been testing the Angular Material renderer set quite thoroughly, and will be contributing some missing features, especially to the Angular Material renderer set.
Description
The Angular Material renderer set renders enums using the
AutocompleteControlRenderer.Unlike the other renderer sets,
AutocompleteControlRendererdoes not translate the enum values.This PR updates
AutocompleteControlRendererto usemapStateToEnumControlProps(JsonFormsState, OwnPropsOfControl & OwnPropsOfEnum)like the other renderer sets, which translates the enum values as labels, while keeping the original values as the values:jsonforms/packages/core/src/mappers/renderer.ts
Lines 676 to 707 in e642b3d
For backwards compatibility, I've allowed the
options@Input to still take instring[](by typing it asEnumOption[] | string[]), but it will not be translated if astring[]is passed.