Picker multi select support #871
Conversation
src/components/picker/index.js
Outdated
| if (this.isDialogAndMultiSelectInEditing()) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Why are we returning undefined here..?
There was a problem hiding this comment.
We don't want to update the outer-textfield with the selection while the user selecting, until they tap on 'Done'
There was a problem hiding this comment.
Ohh ok.
I think that we need to solve the root cause of this issue.
By returning "undefined" we're just hiding the value, but technically it's still being store in the state's value.
I think the right place to handle this issue is in the toggleItemSelection function.
There was a problem hiding this comment.
I'm thinking out loud:
there is a single state to represent the selected items.
then callbacks are called upon user events and then, it invokes with the current state value.
The only place reacting to those state changes is this label, and I didn't want to make extra state value to hold the same data.
I can make it more intuitive by passing to getLabel() the source it should work on (props.value / state.value).
WDYT?
There was a problem hiding this comment.
But fixing the getLabel doesn't fix the source of truth in this case. the value in state.
Technically if in the future we'll add a new feature that depends on the state value, this feature will be broken because it will depend on a wrong value.
I believe that by fixing what we store in the state's value we're fixing the root problem
and not worry about whatever logic depends on it.
Support Picker multi-selection