feat(sidenav): add disableClose option #2501
Conversation
* Adds an attribute to the `md-sidenav` component, which allows developers to disable the closing behavior (e.g escape closing) Backdrop stays separate because you can disable it by using the `mode` attribute. Closes angular#2462
googlebot
added
the
cla: yes
jelbourn
requested review from
mmalerba
and removed request for
jelbourn and
andrewseguin
|
@jelbourn I still like the idea of escapeAction and backdropAction that have values of "none", "close", or a callback function that can be used to implement custom behavior. I remember you being opposed to this when I brought it up last time, but can you remind me why? It seems like a more flexible solution to me. |
|
@mmalerba I mainly was put off by the "provide a callback" API style, since we don't do that anywhere else in the library. So rather than having |
|
Couldn't we just start using this pattern here and elsewhere where it seems appropriate? The API seems clunky otherwise. You need disableCloseOnEscape, disableCloseOnBackdropClick, backdropClick, and escapePressed in order to give users the same amount of control. I think in these cases where the user wants to be able to stop the default action and supply their own the callback style API really makes more sense. |
|
I think I'm outnumbered/outranked on this one, go ahead and do what Jeremy is suggesting |
|
sorry didn't mean to close |
| @Input() | ||
| get disableClose(): boolean { return this._disableClose; } | ||
| set disableClose(value: boolean) { this._disableClose = coerceBooleanProperty(value); } | ||
| private _disableClose: boolean = false; |
There was a problem hiding this comment.
disableClose should also prevent close from backdrop clicks.
While you're doing this, can you also change the @Output to backdropClick? Looks like we missed it when getting rid of dash-case stuff.
There was a problem hiding this comment.
@jelbourn Made the changes you requested.
|
LGTM |
jelbourn
added
pr: lgtm
action: merge
| @@ -414,7 +462,7 @@ class SidenavContainerTwoSidenavTestApp { } | |||
| /** Test component that contains an MdSidenavContainer and one MdSidenav. */ | |||
| @Component({ | |||
| template: ` | |||
| <md-sidenav-container (backdrop-clicked)="backdropClicked()"> | |||
| <md-sidenav-container (backdropClick)="backdropClicked()"> | |||
There was a problem hiding this comment.
@maxime1992 The solution to our problem : backdropClick
|
I think the documentation is not up to date on the Event emitted when the sidenav backdrop is clicked.
Now, for use event of backdrop-clicked, it's backdropClick as I see. We don't have any hook if user click on escape. |
|
There's no class in the html added to indicate that the container is disabled on disable close. This is important for ui tweaks (psuedo selectors - X in the corner ) to tell the user whether they can close the modal or not via outside click. |
|
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. |
Adds an attribute to the
md-sidenavcomponent, which allows developers to disable the closing behavior (e.g escape closing)Backdrop stays separate because you can disable it by using the
modeattribute.Closes #2462