fix(sidenav): unable to close by pressing escape in side mode

[crisbeto]

We currently listen for keydown events on the sidenav so that we know when to close it, however in side mode we don't trap focus inside the sidenav which means that unless the user goes into the sidenav manually, the escape listener won't work. These changes fix the issue by listening to keyboard events at the document level.

Fixes #17965.

[mmalerba]

Is this actually the behavior we want? I'm not sure it makes sense to close the sidenav on escape if the focus isn't somewhere in the sidenav

[crisbeto]

I suppose that it depends on how we treat the sidenav in side mode: is it something that's supposed to be open temporarily, or it should always be open? If it's the latter, I can see it conflicting with other components, otherwise I think that it makes sense to handle events that come from the outside.

[mmalerba]

I think it might depend on individual apps, I could see wanting to do it either way. With the other modes they're modal so its a little more clear. I think we should just leave it as is, and let users add an extra listener if they want it to close on escape in a different subtree

[crisbeto]

Up to you. I submitted the fix, because the behavior felt inconsistent.

[mmalerba]

Ok, I would prefer to leave it as is

[alexbjorlig]

Is it somehow possible to capture focus, when opening the sidenav in "side" mode?

I agree that it should only close on escape, if focus is on the sidenav. But is that not the case with the autoFocus property? I'm asking because using autoFocus and escape in side mode, does not seem to fix the problem.

[crisbeto]

Good point that autoFocus is ignored in side mode on purpose (see https://github.com/angular/components/blob/master/src/material/sidenav/drawer.ts#L258). You could move focus yourself inside the closed event, but that's also not very clean. @mmalerba maybe we should have it always capture focus if autoFocus is set explicitly?

[mmalerba]

I'm not sure about turning on focus trapping because they specified autoFocus, that seems kind of unexpected. But I could see still auto-focusing the first element even though trapping isn't enabled

[crisbeto]

Makes sense. I can do a separate PR tomorrow which adds that functionality.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}