From ea977cf1ceac9b74fb1789bf8f72bfe1d3c72b03 Mon Sep 17 00:00:00 2001
From: Westbrook Johnson <westbrook.johnson@gmail.com>
Date: Mon, 6 Jul 2020 22:33:21 -0400
Subject: [PATCH] fix(sidenav): manage tabindex when interacting with keyboard

---
 packages/sidenav/src/Sidenav.ts       | 24 ++++++++++
 packages/sidenav/src/SidenavItem.ts   | 26 +++++-----
 packages/sidenav/test/sidenav.test.ts | 69 ++++++++++++++++++++++++++-
 3 files changed, 107 insertions(+), 12 deletions(-)

diff --git a/packages/sidenav/src/Sidenav.ts b/packages/sidenav/src/Sidenav.ts
index ce4c178748..8d125075a2 100644
--- a/packages/sidenav/src/Sidenav.ts
+++ b/packages/sidenav/src/Sidenav.ts
@@ -80,10 +80,30 @@ export class SideNav extends Focusable {
 
     private startListeningToKeyboard = (): void => {
         this.addEventListener('keydown', this.handleKeydown);
+        /* istanbul ignore else */
+        if (this.value) {
+            const selected = this.querySelector(
+                `[value="${this.value}"]`
+            ) as SideNavItem;
+            /* istanbul ignore else */
+            if (selected) {
+                selected.tabIndex = -1;
+            }
+        }
     };
 
     private stopListeningToKeyboard = (): void => {
         this.removeEventListener('keydown', this.handleKeydown);
+        /* istanbul ignore else */
+        if (this.value) {
+            const selected = this.querySelector(
+                `[value="${this.value}"]`
+            ) as SideNavItem;
+            /* istanbul ignore else */
+            if (selected) {
+                selected.tabIndex = 0;
+            }
+        }
     };
 
     private handleKeydown(event: KeyboardEvent): void {
@@ -141,6 +161,10 @@ export class SideNav extends Focusable {
     protected firstUpdated(changes: PropertyValues): void {
         super.firstUpdated(changes);
         this.tabIndex = 0;
+        const selectedChild = this.querySelector('[selected]') as SideNavItem;
+        if (selectedChild) {
+            this.value = selectedChild.value;
+        }
     }
 
     protected updated(changes: PropertyValues): void {
diff --git a/packages/sidenav/src/SidenavItem.ts b/packages/sidenav/src/SidenavItem.ts
index f21ac296dc..5da619776e 100644
--- a/packages/sidenav/src/SidenavItem.ts
+++ b/packages/sidenav/src/SidenavItem.ts
@@ -87,21 +87,25 @@ export class SideNavItem extends LikeAnchor(Focusable) {
             if (this.hasChildren) {
                 this.expanded = !this.expanded;
             } else if (this.value) {
-                const selectDetail: SidenavSelectDetail = {
-                    value: this.value,
-                };
-
-                const selectionEvent = new CustomEvent('sidenav-select', {
-                    bubbles: true,
-                    composed: true,
-                    detail: selectDetail,
-                });
-
-                this.dispatchEvent(selectionEvent);
+                this.announceSelected(this.value);
             }
         }
     }
 
+    private announceSelected(value: string): void {
+        const selectDetail: SidenavSelectDetail = {
+            value,
+        };
+
+        const selectionEvent = new CustomEvent('sidenav-select', {
+            bubbles: true,
+            composed: true,
+            detail: selectDetail,
+        });
+
+        this.dispatchEvent(selectionEvent);
+    }
+
     public click(): void {
         this.handleClick();
     }
diff --git a/packages/sidenav/test/sidenav.test.ts b/packages/sidenav/test/sidenav.test.ts
index a6a75df15a..dc5c283fe2 100644
--- a/packages/sidenav/test/sidenav.test.ts
+++ b/packages/sidenav/test/sidenav.test.ts
@@ -14,7 +14,13 @@ import '../sp-sidenav.js';
 import '../sp-sidenav-item.js';
 import '../sp-sidenav-heading.js';
 import { SideNav, SideNavItem } from '../';
-import { fixture, elementUpdated, html, expect } from '@open-wc/testing';
+import {
+    fixture,
+    elementUpdated,
+    html,
+    expect,
+    waitUntil,
+} from '@open-wc/testing';
 import { TemplateResult } from 'lit-html';
 import { LitElement } from 'lit-element';
 
@@ -166,6 +172,67 @@ describe('Sidenav', () => {
 
         expect(el.value).to.equal('Section 2a');
     });
+    it('prevents [tabindex=0] while `focusin`', async () => {
+        const el = await fixture<SideNav>(
+            html`
+                <sp-sidenav manage-tab-index>
+                    <sp-sidenav-heading label="CATEGORY 1">
+                        <sp-sidenav-item
+                            value="Section 0"
+                            label="Section 0"
+                        ></sp-sidenav-item>
+                        <sp-sidenav-item
+                            value="Section 1"
+                            label="Section 1"
+                            selected
+                        ></sp-sidenav-item>
+                        <sp-sidenav-item
+                            value="Section 2"
+                            label="Section 2"
+                            disabled
+                        ></sp-sidenav-item>
+                        <sp-sidenav-item value="Section 3" label="Section 3">
+                            <sp-sidenav-item
+                                value="Section 3a"
+                                label="Section 3a"
+                            ></sp-sidenav-item>
+                        </sp-sidenav-item>
+                    </sp-sidenav-heading>
+                </sp-sidenav>
+            `
+        );
+        const selected = el.querySelector('[value="Section 1"]') as SideNavItem;
+        const toBeSelected = el.querySelector(
+            '[value="Section 0"]'
+        ) as SideNavItem;
+
+        await elementUpdated(el);
+        await waitUntil(() => el.value === 'Section 1', 'wait for selection');
+
+        expect(el.value).to.equal('Section 1');
+        expect(selected.tabIndex).to.equal(0);
+
+        el.dispatchEvent(new Event('focusin'));
+
+        await elementUpdated(el);
+
+        expect(el.value).to.equal('Section 1');
+        expect(selected.tabIndex).to.equal(-1);
+
+        el.dispatchEvent(new Event('focusout'));
+
+        await elementUpdated(el);
+
+        expect(el.value).to.equal('Section 1');
+        expect(selected.tabIndex).to.equal(0);
+
+        toBeSelected.click();
+
+        await elementUpdated(el);
+
+        expect(el.value).to.equal('Section 0');
+        expect(toBeSelected.tabIndex).to.equal(0);
+    });
     it('manage tab index', async () => {
         const el = await fixture<SideNav>(
             html`