Implement the select component edit form #60
Conversation
sergei-maertens
force-pushed
the
feature/59-select-component
branch
4 times, most recently
from
17c9651 to
9f2ada3
Compare
|
I am experiencing a weird behavior with datasource:
|
| useLayoutEffect(() => { | ||
| if (dataSrc !== 'variable' || isEqual(defaultValue, {})) return; | ||
| setFieldValue('defaultValue', ''); | ||
| }, [dataSrc]); |
There was a problem hiding this comment.
Should we watch for defaultValue as well?
There was a problem hiding this comment.
I don't think so, because the reset behaviour should only kick in when the dataSrc value changes.
If the dataSrc is set to manual and the user changes the default value in the dropdown, this effect would cause it to be reset to no default value which entirely beats the point of having a default value field :P
There is a bug here however - the comparison should be against '' instead of {} in the guard clause.
The idea is that whenever the user:
- switch from variable -> manual: nothing to do, since it's not possible to set a default value for 'variable' dataSrc
- switch from manual -> variable: if a default value was set, clear it, since we don't know which values will exist (they come from
itemsExpression). if no default value was set -> nothing to do/reset
There was a problem hiding this comment.
This made me realize that the multiple behaviour was also not incorporated - which is not relevant for selectboxes or radio, so I've included that in my latest commit.
There was a problem hiding this comment.
But what about ESLint in this case? As you said it requires every variable used to be watched?
There was a problem hiding this comment.
yeah, this would be one case where you silence the ESLint warning - complying with the ESLint rule would lead to incorrect behaviour in this case.
It's not a perfect linter rule, unfortunately. The goal of React was also to figure out the dependencies at compile time, but they can't do so reliably (yet) because of usages like this.
We have another case like this in the SDK, where progress indicator on mobile should collapse on navigate: https://github.com/open-formulieren/open-forms-sdk/blob/8117bacd777d785b197703cc9948bce9c9e004fc/src/components/ProgressIndicator/index.js#L30
Letting the hook run when expanded changes there basically prevents you from ever opening the progress indicator. Sometimes a certain state should deliberately be excluded from the hook triggers.
It's a CSS issue with bootstrap/formio that applies a dumb z-index to the Tracking this in #67 |
Mostly copy and paste from radio component, with the changes for data.values rather than the values key in the root.
The hasOwnProperty could not deal with dotted paths, which caused stale data to be present. This in turn triggered validation errors, preventing the edit model from being submittable.
* Added documentation on utility function * Fixed equality check on defaultValue reset The equality check made me realize that the component supports multiple, so the empty value depends on how the component is configured. It is either string or string[], so the empty values are empty string or empty array.
sergei-maertens
force-pushed
the
feature/59-select-component
branch
from
4f72764 to
04affe5
Compare
Closes #59