fix(datepicker): multiple dialog open if the user holds down enter key #12238
Conversation
googlebot
added
the
cla: yes
src/lib/datepicker/datepicker.ts
Outdated
| @@ -380,6 +380,10 @@ export class MatDatepicker<D> implements OnDestroy, CanColor { | |||
|
|
|||
| /** Open the calendar as a dialog. */ | |||
| private _openAsDialog(): void { | |||
| if (this._dialogRef) { | |||
There was a problem hiding this comment.
I'm curious, why is this needed? It looks like in the open method where this is called from we already set a variable to make sure it only gets opened once. Is that not working for some reason?
There was a problem hiding this comment.
I think that when you're holding the key down, eventually some of them go through, because we reset some of the variables in a setTimeout. You can see in the gif how it can take a little bit of holding before the dialogs start stacking.
EDIT: I think it might also have to do with the fact that we change _opened variable in a callback to dialogRef.afterClosed which fires once the animation is finished.
There was a problem hiding this comment.
This change feels like masking the problem rather than fixing it. What if we switch to doing it in beforeClosed instead?
There was a problem hiding this comment.
That one doesn't help much either. The only difference is that beforeClose is fired when the animation starts (which is still async) and the other one fires when the animation is done. IMO doing this is fine since it's pretty low maintenance on our end and it handles an edge case.
There was a problem hiding this comment.
Ok, as long as we've tried other solutions. Can you add a comment explaining why this is necessary even though it looks like it would be handled above?
Fixes the case where the user might be holding down the enter key for a datepicker in touch mode, which could cause it to open multiple dialogs at the same time.
crisbeto
force-pushed
the
datepicker-multiple-dialog
branch
from
b5e88ae to
ad9a032
Compare
mmalerba
added
pr: lgtm
action: merge
#12238) Fixes the case where the user might be holding down the enter key for a datepicker in touch mode, which could cause it to open multiple dialogs at the same time.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes the case where the user might be holding down the enter key for a datepicker in touch mode, which could cause it to open multiple dialogs at the same time.
For reference:
