AAE-22947 Dropdown widget code should be refactored #9794
Conversation
wiktord2000
requested review from
eromano,
popovicsandras and
DenysVuika
as code owners
|
This PR includes some code improvements, especially I decided to store directly {id: 'option_id', name: 'Option'} during option selection instead of keeping only 'option_id' and next finding corresponding option from options if needed. Indeed we had inconsistency between multiple and single mode. When selecting multiple we store array of objects e.g. [{id: 'option_id', name: 'Option'}, {id: 'option_id2', name: 'Option2'}] and in single only the id of selected value. I tried to simplify this code but I'm not sure whether I didn't change to much, so please try to review in detail. Note: We always save (before and now) entire object of the selected option e.g. {id: 'option_id', name: 'Option'} in the form lvl (instead of id) during save/complete. Hence we needed some magic every time we ware changing selected option in single mode. |
lib/core/src/lib/form/components/widgets/core/form-field-validator.spec.ts
Outdated
Show resolved
Hide resolved
lib/core/src/lib/form/components/widgets/core/form-field.model.ts
Outdated
Show resolved
Hide resolved
| if (this.hasEmptyValue === undefined) { | ||
| this.hasEmptyValue = json?.hasEmptyValue ?? !!emptyOption; | ||
| } | ||
| this.updateForm(); |
There was a problem hiding this comment.
was this already inside the get value? please test this code very well it could break several things
There was a problem hiding this comment.
Need more info, don't get it
| } | ||
| if (this.hasEmptyValue && value === null) { | ||
| value = this.emptyValueOption; | ||
| return value; |
There was a problem hiding this comment.
why do u return assign and return value instead of just returning this.emptyOption?
There was a problem hiding this comment.
Thx, forgot to change it
| @@ -285,7 +294,7 @@ export class DropdownCloudWidgetComponent extends WidgetComponent implements OnI | |||
| } | |||
|
|
|||
| private resetValue() { | |||
| this.field.value = ''; | |||
| this.field.value = null; | |||
There was a problem hiding this comment.
this sounds very scary. until now we were resetting the field value with an empty string ( so it will never be null ) and now u are passing null, why?
There was a problem hiding this comment.
I thought that since value is indeed of type FormFieldOption (not id - string as before) I can switch to null
wiktord2000
force-pushed
the
fix/AAE-22947-readonly-dropdown-should-not-display-errors
branch
2 times, most recently
from
8c2303b
to
c4d7f5c
Compare
wiktord2000
changed the title
wiktord2000
changed the title
wiktord2000
force-pushed
the
fix/AAE-22947-readonly-dropdown-should-not-display-errors
branch
from
779cd9a
to
6b4ad04
Compare
|
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x")
What is the current behaviour? (You can also link to an open issue here)
https://hyland.atlassian.net/browse/AAE-22947
What is the new behaviour?
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: