docs(material/sidenav): Document cdkFocusInitial for sidenav #19564

ghost · 2020-06-08T09:56:51Z

adds documentation about focusing inside the sidenav component
Fixes #19450

adds documentation about focusing inside the sidenav component Fixes #19450

src/material/sidenav/sidenav.md

crisbeto

LGTM

…ocs-initial-focus-sidenav

mmalerba · 2020-06-08T17:17:50Z

src/material/sidenav/sidenav.md

+#### Focus management
+The sidenav captures focus automatically by default, unless it's in the side mode. Focus capturing can be enabled or disabled explicitly, using the `autoFocus` input.
+
+By default the first tabbable element will recieve focus upon open. If you want a different element to be focused, you can set the cdkFocusInitial attribute on it.


nit: Please add backquotes around cdkFocusInitial

mmalerba · 2020-06-08T17:22:06Z

src/material/sidenav/sidenav.md

@@ -209,6 +209,11 @@ Similarly, the `<mat-sidenav-content>` should be given a role based on what it c
 represents the primary content of the page, it may make sense to mark it `role="main"`. If no more
 specific role makes sense, `role="region"` is again a good fallback.

+#### Focus management
+The sidenav captures focus automatically by default, unless it's in the side mode. Focus capturing can be enabled or disabled explicitly, using the `autoFocus` input.


How do you feel about this? I think it makes it just a little bit easier to follow

The sidenav has the ability to capture focus automatically. This behavior is turned on by default for the push and over mode sidenavs, while it is off by default for the side mode sidenav. You can set it explicitly regardless of the mode using the autoFocus input.

I think, the older version is a bit laconic. Also it only talks about autoFocus property, without adding knowledge about sidenav mode definitions. Said so, you're version is better in flow. The sentences flow one after another smoothly.
In conclusion, I would still leave my version :)

In particular, I don't like the phrase "automatically by default". I understand what you're trying to say, but its confusing because people often use the "automatically" to mean "by default".

only talks about autoFocus property, without adding knowledge about sidenav mode definitions

I think we should mention the other modes explicitly, its easier to understand when you tell people what modes do have this behavior rather than (or in addition to) which ones don't.

The sidenav has the ability to capture focus. This behavior is turned on for push and over modes and it is off for side mode. you can change its default behavior by autoFocus input

what do you think about that?

I like that. It's succinct and addresses the concerns I had

googlebot · 2020-07-14T14:06:37Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-07-14T14:17:35Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mmalerba

LGTM

…#19564) * docs(material/sidenav): Document cdkFocusInitial for sidenav adds documentation about focusing inside the sidenav component Fixes angular#19450 * cdkFocusInitial sentence rephrased * sidenav autoFocus explained in details * sidenav focus statement rephrased * gramma error fixed * rephrased autoFocus sentence * Update sidenav.md Co-authored-by: mmalerba <mmalerba@google.com>

angular-automatic-lock-bot · 2020-09-04T00:09:36Z

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.}

docs(material/sidenav): Document cdkFocusInitial for sidenav

448c127

adds documentation about focusing inside the sidenav component Fixes #19450

ghost requested a review from mmalerba as a code owner June 8, 2020 09:56

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 8, 2020

crisbeto reviewed Jun 8, 2020

View reviewed changes

src/material/sidenav/sidenav.md Outdated Show resolved Hide resolved

src/material/sidenav/sidenav.md Outdated Show resolved Hide resolved

lukonik added 2 commits June 8, 2020 14:14

cdkFocusInitial sentence rephrased

b1a98f1

sidenav autoFocus explained in details

060b7d9

crisbeto reviewed Jun 8, 2020

View reviewed changes

src/material/sidenav/sidenav.md Outdated Show resolved Hide resolved

sidenav focus statement rephrased

1f5b4eb

crisbeto reviewed Jun 8, 2020

View reviewed changes

src/material/sidenav/sidenav.md Outdated Show resolved Hide resolved

gramma error fixed

17e4c4f

crisbeto approved these changes Jun 8, 2020

View reviewed changes

Merge branch 'master' of https://github.com/angular/components into d…

b78c305

…ocs-initial-focus-sidenav

mmalerba reviewed Jun 8, 2020

View reviewed changes

mmalerba removed the action: merge The PR is ready for merge by the caretaker label Jun 8, 2020

googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Jul 14, 2020

rephrased autoFocus sentence

50fa77b

googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jul 14, 2020

ghost requested a review from mmalerba July 14, 2020 14:39

Update sidenav.md

2fe17f6

mmalerba added the action: merge The PR is ready for merge by the caretaker label Jul 14, 2020

mmalerba approved these changes Jul 14, 2020

View reviewed changes

andrewseguin added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jul 17, 2020

andrewseguin merged commit 60ce108 into angular:master Jul 17, 2020

mmalerba added docs This issue is related to documentation and removed docs This issue is related to documentation labels Aug 4, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 4, 2020

ghost deleted the docs-initial-focus-sidenav branch September 4, 2020 07:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs(material/sidenav): Document cdkFocusInitial for sidenav #19564

docs(material/sidenav): Document cdkFocusInitial for sidenav #19564

Uh oh!

ghost commented Jun 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crisbeto left a comment

Uh oh!

mmalerba Jun 8, 2020

Uh oh!

mmalerba Jun 8, 2020

Uh oh!

ghost Jun 21, 2020

Uh oh!

mmalerba Jun 23, 2020

Uh oh!

ghost Jun 23, 2020 •

edited by ghost

Loading

Uh oh!

mmalerba Jun 25, 2020

Uh oh!

googlebot commented Jul 14, 2020

Uh oh!

googlebot commented Jul 14, 2020

Uh oh!

mmalerba left a comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 4, 2020

Uh oh!

Uh oh!

docs(material/sidenav): Document cdkFocusInitial for sidenav #19564

docs(material/sidenav): Document cdkFocusInitial for sidenav #19564

Uh oh!

Conversation

ghost commented Jun 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

mmalerba Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

mmalerba Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

ghost Jun 21, 2020

Choose a reason for hiding this comment

Uh oh!

mmalerba Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

ghost Jun 23, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmalerba Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

googlebot commented Jul 14, 2020

Uh oh!

googlebot commented Jul 14, 2020

Uh oh!

mmalerba left a comment

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 4, 2020

Uh oh!

Uh oh!

ghost Jun 23, 2020 •

edited by ghost

Loading