Skip to content
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

MatSidenav with position="end" is in the wrong tab order #15247

Closed
bsteffl opened this issue Feb 20, 2019 · 3 comments · Fixed by #18101
Closed

MatSidenav with position="end" is in the wrong tab order #15247

bsteffl opened this issue Feb 20, 2019 · 3 comments · Fixed by #18101
Assignees
Labels
Accessibility This issue is related to accessibility (a11y) area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@bsteffl
Copy link

bsteffl commented Feb 20, 2019

Bug, feature request, or proposal:

Sidenav doesn't seem to meet https://www.w3.org/WAI/WCAG21/Understanding/meaningful-sequence.html and https://www.w3.org/WAI/WCAG21/Understanding/focus-order.html. Visual order should generally match DOM order and tab sequence.

What is the expected behavior?

Being able to position sidenav either before or after the main content.

What is the current behavior?

Sidenav always appears first in DOM and tab order even if positioned after the content.

Looking at this example you will notice that as you tab from the url to the page it goes from the right panel (Link2) to the main content (Link1), rather than the other way around.
sidenav-tab-order

What are the steps to reproduce?

Providing a StackBlitz reproduction is the best way to share your issue.

StackBlitz starter: https://goo.gl/wwnhMV

https://stackblitz.com/edit/angular-xww88d

What is the use-case or motivation for changing an existing behavior?

To meet WCAG 2.1 requirements

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Material 7.3.1

Is there anything else we should know?

Would be nice if this allowed users the option to control order in the DOM. It may need to change depending on the sidenav mode (over, push, side).

@josephperrott josephperrott added Accessibility This issue is related to accessibility (a11y) P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jun 4, 2019
@crisbeto crisbeto added the has pr label Jan 4, 2020
@crisbeto crisbeto self-assigned this Jan 4, 2020
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 4, 2020
We project all sidenavs before the content in the DOM since we can't know ahead of time what their position will be. This is problematic when the drawer is in the end position, because the visual order of the content no longer matches the tab order. These changes fix the issue by moving the sidenav after the content manually when it's set to `end` and then moving it back if it's set to `start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position` value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're moving the element around since the anchors will be left at their old positions. To avoid adding extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
@nehaabrol87
Copy link

Thanks @crisbeto !
I will watch this issue and the PR.
I did not know that this had already been opened.
This will be a create add on for
#18100

@qwad1000
Copy link

qwad1000 commented Jan 6, 2020

This also seems to be relevant issue #11992

crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 19, 2020
We project all sidenavs before the content in the DOM since we can't know ahead of time what their position will be. This is problematic when the drawer is in the end position, because the visual order of the content no longer matches the tab order. These changes fix the issue by moving the sidenav after the content manually when it's set to `end` and then moving it back if it's set to `start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position` value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're moving the element around since the anchors will be left at their old positions. To avoid adding extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jul 4, 2020
We project all sidenavs before the content in the DOM since we can't know ahead of time what their position will be. This is problematic when the drawer is in the end position, because the visual order of the content no longer matches the tab order. These changes fix the issue by moving the sidenav after the content manually when it's set to `end` and then moving it back if it's set to `start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position` value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're moving the element around since the anchors will be left at their old positions. To avoid adding extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue Oct 15, 2020
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
@jelbourn jelbourn changed the title Bug - MatSidenav always appears first in tab order MatSidenav with position="end" is in the wrong tab order Nov 25, 2020
crisbeto added a commit to crisbeto/material2 that referenced this issue Nov 26, 2020
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue May 6, 2021
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue Aug 16, 2021
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue Dec 9, 2021
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
crisbeto added a commit to crisbeto/material2 that referenced this issue Jan 14, 2022
…sual order

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
andrewseguin pushed a commit that referenced this issue Jan 18, 2022
…sual order (#18101)

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes #15247.
andrewseguin pushed a commit that referenced this issue Jan 18, 2022
…sual order (#18101)

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes #15247.

(cherry picked from commit c489f37)
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 18, 2022
forsti0506 pushed a commit to forsti0506/components that referenced this issue Apr 3, 2022
…sual order (angular#18101)

We project all sidenavs before the content in the DOM since we can't know ahead of time what their
position will be. This is problematic when the drawer is in the end position, because the visual
order of the content no longer matches the tab order. These changes fix the issue by moving the
sidenav after the content manually when it's set to `end` and then moving it back if it's set to
`start` again.

A couple of notes:
1. We could technically do this with content projection, but it would only work when the `position`
value is static (e.g. `position="end"`). I did it this way so we can cover the case where it's
data bound.
2. Currently the focus trap anchors are set around the sidenav, but that's problematic when we're
moving the element around since the anchors will be left at their old positions. To avoid adding
extra logic for moving the anchors, I've moved the focus trap to be inside the sidenav. Here's
what the DOM looks like at the moment:

```html
<container>
  <anchor/>
  <sidenav>Content</sidenav>
  <anchor/>
</container>
```

And this is what I've changed it to:
```html
<container>
  <sidenav>
    <anchor/>
    Content
    <anchor/>
  </sidenav>
</container
```

Fixes angular#15247.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: material/sidenav P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants