Picker dialog dismiss bug #3858
Conversation
yotam-wix
requested review from
adids1221 and
lidord-wix
and removed request for
adids1221
lidord-wix
requested review from
M-i-k-e-l
and removed request for
lidord-wix
|
@yotam-wix please merge master after the monorepo change 🙏 |
yotam-wix
force-pushed
the
picker-dialog-dismiss-bug
branch
from
634a5c1 to
ceedda9
Compare
|
#rebuild |
@M-i-k-e-l done |
|
@yotam-wix you forgot to assign it to me, I'll look at it next week |
| ref={pickerExpandable} | ||
| useDialog={useDialog || useWheelPicker} | ||
| dialogProps={DEFAULT_DIALOG_PROPS} | ||
| dialogProps={{...DEFAULT_DIALOG_PROPS, onDismiss: handleCancelAction}} |
There was a problem hiding this comment.
You're creating a new object each render
There was a problem hiding this comment.
@M-i-k-e-l Would a better solution be to create a memoized value using useMemo for this object of dialogProps that re-calculates for each change in handleCancelAction?
|
|
||
| const cancelSelect = useCallback(() => { | ||
| handleCancelAction(); | ||
| pickerExpandableRef.current?.closeExpandable?.(); |
There was a problem hiding this comment.
This fix changes the order of topBarProps?.onCancel?.(); and pickerExpandableRef.current?.closeExpandable?.();.
Although I could not find a bug with it, it does not mean it does not break anything.
I am not saying that we have to find another solution but we should be aware of this and possibly try to find a solution that does not change the order.
| [multiDraftValue]); | ||
|
|
||
| const cancelSelect = useCallback(() => { | ||
| const handleCancelAction = useCallback(() => { |
There was a problem hiding this comment.
I don't really know what is the difference between handleCancelAction and cancelSelect (name wise), try to think of a better name
There was a problem hiding this comment.
will change it to resetSelectionState if that makes sense
Description
Fixed Picker dialog bug where on dismiss it 'remembers' the selected values.
Changelog
Picker - Fixed dialog onDismiss bug.
Additional info
Ticket 4680