fix(material/sidenav): end position sidenav tab order not matching visual order

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>;