New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(cdk/overlay): connected-overlay directive should have input for disabling escape close #19420 #20585
feat(cdk/overlay): connected-overlay directive should have input for disabling escape close #19420 #20585
Conversation
f081f94
to
8d1803b
Compare
@@ -181,6 +181,9 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges { | |||
/** Whether the overlay is open. */ | |||
@Input('cdkConnectedOverlayOpen') open: boolean = false; | |||
|
|||
/** Whether the overlay is open. */ | |||
@Input('cdkConnectedOverlayCloseOnEscape') closeOnEscape: boolean = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have something similar in the dialog, bottom sheet and sidenav called disableClose
which we should probably align with. Note that in the other components it means disabling close when pressing escape and clicking on the backdrop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, we should be consistent with the name and the behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't know about disableClose
, totally agree with it, I will modify PR soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
541cd6b
to
9e84c0e
Compare
…disabling escape close
9e84c0e
to
ab9d94f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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. |
#19420