fix(material/sidenav): end position sidenav tab order not matching visual 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.

Author: crisbeto

src/material/sidenav/drawer.html

@@ -1,3 +1,3 @@
-<div class="mat-drawer-inner-container" cdkScrollable>
+<div class="mat-drawer-inner-container" cdkScrollable #content>
   <ng-content></ng-content>
 </div>

src/material/sidenav/drawer.spec.ts

@@ -661,6 +661,144 @@ describe('MatDrawer', () => {
     expect(scrollable).toBeTruthy();
     expect(scrollable.getElementRef().nativeElement).toBe(content.nativeElement);
   });
+
+  describe('DOM position', () => {
+    it('should project start drawer before the content', () => {
+      const fixture = TestBed.createComponent(BasicTestApp);
+      fixture.componentInstance.position = 'start';
+      fixture.detectChanges();
+
+      const allNodes = getDrawerNodesArray(fixture);
+      const drawerIndex = allNodes.indexOf(fixture.nativeElement.querySelector('.mat-drawer'));
+      const contentIndex = allNodes.indexOf(
+        fixture.nativeElement.querySelector('.mat-drawer-content'),
+      );
+
+      expect(drawerIndex)
+        .withContext('Expected drawer to be inside the container')
+        .toBeGreaterThan(-1);
+      expect(contentIndex)
+        .withContext('Expected content to be inside the container')
+        .toBeGreaterThan(-1);
+      expect(drawerIndex)
+        .withContext('Expected drawer to be before the content')
+        .toBeLessThan(contentIndex);
+    });
+
+    it('should project end drawer after the content', () => {
+      const fixture = TestBed.createComponent(BasicTestApp);
+      fixture.componentInstance.position = 'end';
+      fixture.detectChanges();
+
+      const allNodes = getDrawerNodesArray(fixture);
+      const drawerIndex = allNodes.indexOf(fixture.nativeElement.querySelector('.mat-drawer'));
+      const contentIndex = allNodes.indexOf(
+        fixture.nativeElement.querySelector('.mat-drawer-content'),
+      );
+
+      expect(drawerIndex)
+        .withContext('Expected drawer to be inside the container')
+        .toBeGreaterThan(-1);
+      expect(contentIndex)
+        .withContext('Expected content to be inside the container')
+        .toBeGreaterThan(-1);
+      expect(drawerIndex)
+        .withContext('Expected drawer to be after the content')
+        .toBeGreaterThan(contentIndex);
+    });
+
+    it(
+      'should move the drawer before/after the content when its position changes after being ' +
+        'initialized at `start`',
+      () => {
+        const fixture = TestBed.createComponent(BasicTestApp);
+        fixture.componentInstance.position = 'start';
+        fixture.detectChanges();
+
+        const drawer = fixture.nativeElement.querySelector('.mat-drawer');
+        const content = fixture.nativeElement.querySelector('.mat-drawer-content');
+
+        let allNodes = getDrawerNodesArray(fixture);
+        const startDrawerIndex = allNodes.indexOf(drawer);
+        const startContentIndex = allNodes.indexOf(content);
+
+        expect(startDrawerIndex)
+          .withContext('Expected drawer to be inside the container')
+          .toBeGreaterThan(-1);
+        expect(startContentIndex)
+          .withContext('Expected content to be inside the container')
+          .toBeGreaterThan(-1);
+        expect(startDrawerIndex)
+          .withContext('Expected drawer to be before the content on init')
+          .toBeLessThan(startContentIndex);
+
+        fixture.componentInstance.position = 'end';
+        fixture.detectChanges();
+        allNodes = getDrawerNodesArray(fixture);
+
+        expect(allNodes.indexOf(drawer))
+          .withContext('Expected drawer to be after content when position changes to `end`')
+          .toBeGreaterThan(allNodes.indexOf(content));
+
+        fixture.componentInstance.position = 'start';
+        fixture.detectChanges();
+        allNodes = getDrawerNodesArray(fixture);
+
+        expect(allNodes.indexOf(drawer))
+          .withContext('Expected drawer to be before content when position changes back to `start`')
+          .toBeLessThan(allNodes.indexOf(content));
+      },
+    );
+
+    it(
+      'should move the drawer before/after the content when its position changes after being ' +
+        'initialized at `end`',
+      () => {
+        const fixture = TestBed.createComponent(BasicTestApp);
+        fixture.componentInstance.position = 'end';
+        fixture.detectChanges();
+
+        const drawer = fixture.nativeElement.querySelector('.mat-drawer');
+        const content = fixture.nativeElement.querySelector('.mat-drawer-content');
+
+        let allNodes = getDrawerNodesArray(fixture);
+        const startDrawerIndex = allNodes.indexOf(drawer);
+        const startContentIndex = allNodes.indexOf(content);
+
+        expect(startDrawerIndex).toBeGreaterThan(-1, 'Expected drawer to be inside the container');
+        expect(startContentIndex).toBeGreaterThan(
+          -1,
+          'Expected content to be inside the container',
+        );
+        expect(startDrawerIndex).toBeGreaterThan(
+          startContentIndex,
+          'Expected drawer to be after the content on init',
+        );
+
+        fixture.componentInstance.position = 'start';
+        fixture.detectChanges();
+        allNodes = getDrawerNodesArray(fixture);
+
+        expect(allNodes.indexOf(drawer)).toBeLessThan(
+          allNodes.indexOf(content),
+          'Expected drawer to be before content when position changes to `start`',
+        );
+
+        fixture.componentInstance.position = 'end';
+        fixture.detectChanges();
+        allNodes = getDrawerNodesArray(fixture);
+
+        expect(allNodes.indexOf(drawer)).toBeGreaterThan(
+          allNodes.indexOf(content),
+          'Expected drawer to be after content when position changes back to `end`',
+        );
+      },
+    );
+
+    function getDrawerNodesArray(fixture: ComponentFixture<any>): HTMLElement[] {
+      return Array.from(fixture.nativeElement.querySelector('.mat-drawer-container').childNodes);
+    }
+  });
 });
 
 describe('MatDrawerContainer', () => {
@@ -949,6 +1087,41 @@ describe('MatDrawerContainer', () => {
     expect(spy).toHaveBeenCalled();
     subscription.unsubscribe();
   }));
+
+  it('should position the drawers before/after the content in the DOM based on their position', fakeAsync(() => {
+    const fixture = TestBed.createComponent(DrawerContainerTwoDrawerTestApp);
+    fixture.detectChanges();
+
+    const drawerDebugElements = fixture.debugElement.queryAll(By.directive(MatDrawer));
+    const [start, end] = drawerDebugElements.map(el => el.componentInstance);
+    const [startNode, endNode] = drawerDebugElements.map(el => el.nativeElement);
+    const contentNode = fixture.nativeElement.querySelector('.mat-drawer-content');
+    const allNodes: HTMLElement[] = Array.from(
+      fixture.nativeElement.querySelector('.mat-drawer-container').childNodes,
+    );
+    const startIndex = allNodes.indexOf(startNode);
+    const endIndex = allNodes.indexOf(endNode);
+    const contentIndex = allNodes.indexOf(contentNode);
+
+    expect(start.position).toBe('start');
+    expect(end.position).toBe('end');
+    expect(contentIndex)
+      .withContext('Expected content to be inside the container')
+      .toBeGreaterThan(-1);
+    expect(startIndex)
+      .withContext('Expected start drawer to be inside the container')
+      .toBeGreaterThan(-1);
+    expect(endIndex)
+      .withContext('Expected end drawer to be inside the container')
+      .toBeGreaterThan(-1);
+
+    expect(startIndex)
+      .withContext('Expected start drawer to be before content')
+      .toBeLessThan(contentIndex);
+    expect(endIndex)
+      .withContext('Expected end drawer to be after content')
+      .toBeGreaterThan(contentIndex);
+  }));
 });
 
 /** Test component that contains an MatDrawerContainer but no MatDrawer. */
@@ -971,7 +1144,7 @@ class DrawerContainerTwoDrawerTestApp {
 @Component({
   template: `
     <mat-drawer-container (backdropClick)="backdropClicked()" [hasBackdrop]="hasBackdrop">
-      <mat-drawer #drawer="matDrawer" position="start"
+      <mat-drawer #drawer="matDrawer" [position]="position"
                  (opened)="open()"
                  (openedStart)="openStart()"
                  (closed)="close()"
@@ -997,6 +1170,7 @@ class BasicTestApp {
   closeStartCount = 0;
   backdropClickedCount = 0;
   hasBackdrop: boolean | null = null;
+  position = 'start';
 
   @ViewChild('drawer') drawer: MatDrawer;
   @ViewChild('drawerButton') drawerButton: ElementRef<HTMLButtonElement>;

src/material/sidenav/drawer.ts

@@ -22,6 +22,7 @@ import {DOCUMENT} from '@angular/common';
 import {
   AfterContentChecked,
   AfterContentInit,
+  AfterViewInit,
   ChangeDetectionStrategy,
   ChangeDetectorRef,
   Component,
@@ -153,13 +154,19 @@ export class MatDrawerContent extends CdkScrollable implements AfterContentInit
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
 })
-export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestroy {
+export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy {
   private _focusTrap: FocusTrap;
   private _elementFocusedBeforeDrawerWasOpened: HTMLElement | null = null;
 
   /** Whether the drawer is initialized. Used for disabling the initial animation. */
   private _enableAnimations = false;
 
+  /** Whether the view of the component has been attached. */
+  private _isAttached: boolean;
+
+  /** Anchor node used to restore the drawer to its initial position. */
+  private _anchor: Comment | null;
+
   /** The side that the drawer is attached to. */
   @Input()
   get position(): 'start' | 'end' {
@@ -168,7 +175,12 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
   set position(value: 'start' | 'end') {
     // Make sure we have a valid value.
     value = value === 'end' ? 'end' : 'start';
-    if (value != this._position) {
+    if (value !== this._position) {
+      // Static inputs in Ivy are set before the element is in the DOM.
+      if (this._isAttached) {
+        this._updatePositionInParent(value);
+      }
+
       this._position = value;
       this.onPositionChanged.emit();
     }
@@ -293,6 +305,9 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
   // tslint:disable-next-line:no-output-on-prefix
   @Output('positionChanged') readonly onPositionChanged = new EventEmitter<void>();
 
+  /** Reference to the inner element that contains all the content. */
+  @ViewChild('content') _content: ElementRef<HTMLElement>;
+
   /**
    * An observable that emits when the drawer mode changes. This is used by the drawer container to
    * to know when to when the mode changes so it can adapt the margins on the content.
@@ -448,13 +463,20 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
 
   /** Whether focus is currently within the drawer. */
   private _isFocusWithinDrawer(): boolean {
-    const activeEl = this._doc?.activeElement;
+    const activeEl = this._doc.activeElement;
     return !!activeEl && this._elementRef.nativeElement.contains(activeEl);
   }
 
-  ngAfterContentInit() {
+  ngAfterViewInit() {
+    this._isAttached = true;
     this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement);
     this._updateFocusTrapState();
+
+    // Only update the DOM position when the sidenav is positioned at
+    // the end since we project the sidenav before the content by default.
+    if (this._position === 'end') {
+      this._updatePositionInParent('end');
+    }
   }
 
   ngAfterContentChecked() {
@@ -472,6 +494,8 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
       this._focusTrap.destroy();
     }
 
+    this._anchor?.remove();
+    this._anchor = null;
     this._animationStarted.complete();
     this._animationEnd.complete();
     this._modeChanged.complete();
@@ -567,6 +591,28 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
       this._focusTrap.enabled = this.opened && this.mode !== 'side';
     }
   }
+
+  /**
+   * Updates the position of the drawer in the DOM. We need to move the element around ourselves
+   * when it's in the `end` position so that it comes after the content and the visual order
+   * matches the tab order. We also need to be able to move it back to `start` if the sidenav
+   * started off as `end` and was changed to `start`.
+   */
+  private _updatePositionInParent(newPosition: 'start' | 'end') {
+    const element = this._elementRef.nativeElement;
+    const parent = element.parentNode!;
+
+    if (newPosition === 'end') {
+      if (!this._anchor) {
+        this._anchor = this._doc.createComment('mat-drawer-anchor')!;
+        parent.insertBefore(this._anchor!, element);
+      }
+
+      parent.appendChild(element);
+    } else if (this._anchor) {
+      this._anchor.parentNode!.insertBefore(element, this._anchor);
+    }
+  }
 }
 
 /**

tools/public_api_guard/material/sidenav.md

@@ -6,6 +6,7 @@
 
 import { AfterContentChecked } from '@angular/core';
 import { AfterContentInit } from '@angular/core';
+import { AfterViewInit } from '@angular/core';
 import { AnimationEvent as AnimationEvent_2 } from '@angular/animations';
 import { AnimationTriggerMetadata } from '@angular/animations';
 import { BooleanInput } from '@angular/cdk/coercion';
@@ -48,7 +49,7 @@ export const MAT_DRAWER_DEFAULT_AUTOSIZE: InjectionToken<boolean>;
 export function MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY(): boolean;
 
 // @public
-export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestroy {
+export class MatDrawer implements AfterViewInit, AfterContentChecked, OnDestroy {
     constructor(_elementRef: ElementRef<HTMLElement>, _focusTrapFactory: FocusTrapFactory, _focusMonitor: FocusMonitor, _platform: Platform, _ngZone: NgZone, _interactivityChecker: InteractivityChecker, _doc: any, _container?: MatDrawerContainer | undefined);
     readonly _animationEnd: Subject<AnimationEvent_2>;
     readonly _animationStarted: Subject<AnimationEvent_2>;
@@ -61,6 +62,7 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
     _closeViaBackdropClick(): Promise<MatDrawerToggleResult>;
     // (undocumented)
     _container?: MatDrawerContainer | undefined;
+    _content: ElementRef<HTMLElement>;
     get disableClose(): boolean;
     set disableClose(value: BooleanInput);
     // (undocumented)
@@ -71,7 +73,7 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
     // (undocumented)
     ngAfterContentChecked(): void;
     // (undocumented)
-    ngAfterContentInit(): void;
+    ngAfterViewInit(): void;
     // (undocumented)
     ngOnDestroy(): void;
     readonly onPositionChanged: EventEmitter<void>;