From a7be6391f5084085e853a4b7404638eda40f7eaf Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Fri, 4 Oct 2024 16:18:37 -0700
Subject: [PATCH 1/9] remove render modifier

---
 .../frontend/app/components/user-menu.hbs     |  3 +--
 packages/frontend/app/components/user-menu.js | 21 ++++++++-----------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.hbs b/packages/frontend/app/components/user-menu.hbs
index ab73d3d6bb..fef13c5e39 100644
--- a/packages/frontend/app/components/user-menu.hbs
+++ b/packages/frontend/app/components/user-menu.hbs
@@ -2,7 +2,6 @@
   <nav
     class="user-menu{{if this.isOpen ' is-open'}}"
     aria-label={{t "general.userMenu"}}
-    {{did-insert (set this "element")}}
     {{! template-lint-disable no-invalid-interactive }}
     {{on "keyup" this.keyUp}}
     data-test-user-menu
@@ -25,7 +24,7 @@
     {{#if this.isOpen}}
       <div {{on-click-outside (set this "isOpen" false)}}>
         <ul class="menu">
-          <li tabindex="-1" data-test-item {{did-insert this.focus}}>
+          <li tabindex="-1" data-test-item {{focus}}>
             <LinkToWithAction
               @route="myprofile"
               @action={{set this "isOpen" false}}
diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 5793992745..2d0e2fe6ba 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -1,5 +1,4 @@
 import Component from '@glimmer/component';
-import { schedule } from '@ember/runloop';
 import { service } from '@ember/service';
 import { cached, tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
@@ -9,7 +8,6 @@ export default class UserMenuComponent extends Component {
   @service intl;
   @service currentUser;
   @tracked isOpen = false;
-  @tracked element;
 
   userModel = new TrackedAsyncData(this.currentUser.getModel());
 
@@ -49,26 +47,25 @@ export default class UserMenuComponent extends Component {
     return true;
   }
 
-  handleArrowDown(evt, item) {
-    if (evt.target.tagName.toLowerCase() === 'button') {
+  handleArrowDown(event, item) {
+    if (event.target.tagName.toLowerCase() === 'button') {
       this.isOpen = true;
     } else {
       if (item.nextElementSibling) {
         item.nextElementSibling.querySelector('a').focus();
       } else {
-        // eslint-disable-next-line ember/no-runloop
-        schedule('afterRender', () => {
-          this.element.querySelector('.menu li:nth-of-type(1) a').focus();
-        });
+        item.parentElement.firstElementChild.querySelector('a').focus();
       }
     }
   }
 
   handleArrowUp(item) {
-    if (item.previousElementSibling) {
-      item.previousElementSibling.querySelector('a').focus();
-    } else {
-      this.element.querySelector('.menu li:last-of-type a').focus();
+    if (item) {
+      if (item.previousElementSibling) {
+        item.previousElementSibling.querySelector('a').focus();
+      } else {
+        item.parentElement.lastElementChild.querySelector('a').focus();
+      }
     }
   }
 

From 583b45c2d78c4e75fed2f79e64661b5c4557105d Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Fri, 4 Oct 2024 16:19:24 -0700
Subject: [PATCH 2/9] remove unused focus() method

---
 packages/frontend/app/components/user-menu.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 2d0e2fe6ba..9726cbb63f 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -68,8 +68,4 @@ export default class UserMenuComponent extends Component {
       }
     }
   }
-
-  focus(el) {
-    el.focus();
-  }
 }

From 41f527a90ca78fe23afe42121487ab3179f38a43 Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Fri, 4 Oct 2024 16:20:37 -0700
Subject: [PATCH 3/9] updated lint-todo

---
 packages/frontend/.lint-todo | 2 --
 1 file changed, 2 deletions(-)

diff --git a/packages/frontend/.lint-todo b/packages/frontend/.lint-todo
index af7c8ddb86..66aa5311f6 100644
--- a/packages/frontend/.lint-todo
+++ b/packages/frontend/.lint-todo
@@ -1,6 +1,4 @@
 add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1722902400000|1730682000000|1754006400000|app/components/new-directory-user.hbs
-add|ember-template-lint|no-at-ember-render-modifiers|28|43|28|43|5c127b0b8124a93b18177d7580bcc47dbb8ebbff|1722902400000|1730682000000|1754006400000|app/components/user-menu.hbs
-add|ember-template-lint|no-at-ember-render-modifiers|5|4|5|4|1459921678f69a3a659ce69b1da303bc8e96b104|1725580800000|1728172800000|1730768400000|app/components/user-menu.hbs
 add|ember-template-lint|no-at-ember-render-modifiers|4|2|4|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1722902400000|1730682000000|1754006400000|app/components/user-profile-bio.hbs
 add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|f53982efe02d2bef9e7f12b5b862288c594579c2|1722902400000|1730682000000|1754006400000|app/components/user-profile-bio.hbs
 add|ember-template-lint|no-at-ember-render-modifiers|4|2|4|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1722902400000|1730682000000|1754006400000|app/components/user-profile-roles.hbs

From 333a37a70c037394178af9224a585de5ea819196 Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Mon, 7 Oct 2024 12:02:19 -0700
Subject: [PATCH 4/9] added keyboard navigation test for <UserMenu>

---
 .../frontend/app/components/user-menu.hbs     |  7 +-
 packages/frontend/app/components/user-menu.js | 10 +++
 .../integration/components/user-menu-test.js  | 67 +++++++++++++++++--
 .../tests/pages/components/user-menu.js       | 15 ++++-
 4 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.hbs b/packages/frontend/app/components/user-menu.hbs
index fef13c5e39..60ab2958a2 100644
--- a/packages/frontend/app/components/user-menu.hbs
+++ b/packages/frontend/app/components/user-menu.hbs
@@ -29,12 +29,17 @@
               @route="myprofile"
               @action={{set this "isOpen" false}}
               @queryParams={{hash invalidatetokens=null newtoken=null}}
+              data-test-item-link
             >
               {{t "general.myProfile"}}
             </LinkToWithAction>
           </li>
           <li tabindex="-1" data-test-item>
-            <LinkToWithAction @route="logout" @action={{set this "isOpen" false}}>
+            <LinkToWithAction
+              @route="logout"
+              @action={{set this "isOpen" false}}
+              data-test-item-link
+            >
               {{t "general.logout"}}
             </LinkToWithAction>
           </li>
diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 9726cbb63f..a6befb936b 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -19,6 +19,12 @@ export default class UserMenuComponent extends Component {
   @action
   toggleMenu() {
     this.isOpen = !this.isOpen;
+
+    if (this.isOpen) {
+      setTimeout(() => {
+        document.querySelector('.user-menu .menu a:first-of-type').focus();
+      }, 1);
+    }
   }
 
   @action
@@ -50,6 +56,10 @@ export default class UserMenuComponent extends Component {
   handleArrowDown(event, item) {
     if (event.target.tagName.toLowerCase() === 'button') {
       this.isOpen = true;
+      setTimeout(
+        () => event.target.parentElement.querySelector('.menu a:first-of-type').focus(),
+        1,
+      );
     } else {
       if (item.nextElementSibling) {
         item.nextElementSibling.querySelector('a').focus();
diff --git a/packages/frontend/tests/integration/components/user-menu-test.js b/packages/frontend/tests/integration/components/user-menu-test.js
index 23ff0e2541..01bbf10caf 100644
--- a/packages/frontend/tests/integration/components/user-menu-test.js
+++ b/packages/frontend/tests/integration/components/user-menu-test.js
@@ -1,6 +1,6 @@
 import { module, test } from 'qunit';
 import { setupRenderingTest } from 'frontend/tests/helpers';
-import { render } from '@ember/test-helpers';
+import { render, waitFor } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import component from 'frontend/tests/pages/components/user-menu';
 import a11yAudit from 'ember-a11y-testing/test-support/audit';
@@ -11,6 +11,9 @@ module('Integration | Component | user-menu', function (hooks) {
   setupRenderingTest(hooks);
   setupMirage(hooks);
 
+  // My Profile and Logout
+  const linkCount = 2;
+
   hooks.beforeEach(async function () {
     await setupAuthentication();
   });
@@ -32,7 +35,7 @@ module('Integration | Component | user-menu', function (hooks) {
 
     assert.strictEqual(component.links.length, 0);
     await component.toggle.click();
-    assert.strictEqual(component.links.length, 2);
+    assert.strictEqual(component.links.length, linkCount);
   });
 
   test('down opens menu', async function (assert) {
@@ -40,14 +43,14 @@ module('Integration | Component | user-menu', function (hooks) {
 
     assert.strictEqual(component.links.length, 0);
     await component.toggle.down();
-    assert.strictEqual(component.links.length, 2);
+    assert.strictEqual(component.links.length, linkCount);
   });
 
   test('escape closes menu', async function (assert) {
     await render(hbs`<UserMenu />`);
 
     await component.toggle.down();
-    assert.strictEqual(component.links.length, 2);
+    assert.strictEqual(component.links.length, linkCount);
     await component.toggle.esc();
     assert.strictEqual(component.links.length, 0);
   });
@@ -56,8 +59,62 @@ module('Integration | Component | user-menu', function (hooks) {
     await render(hbs`<UserMenu />`);
 
     await component.toggle.down();
-    assert.strictEqual(component.links.length, 2);
+    assert.strictEqual(component.links.length, linkCount);
+    await component.toggle.click();
+    assert.strictEqual(component.links.length, 0);
+  });
+
+  test('keyboard navigation', async function (assert) {
+    await render(hbs`<UserMenu />`);
+    await component.toggle.click();
+    assert.strictEqual(component.links.length, linkCount, `has ${linkCount} links`);
+
+    // slight delay to allow for proper loading of popup menu
+    await waitFor('.user-menu .menu');
+
+    assert.ok(component.links[0].link.hasFocus, 'first link has focus');
+    assert.notOk(component.links[1].link.hasFocus, 'second link does NOT have focus');
+
+    // regular down/up navigation
+    await component.links[0].down();
+    assert.notOk(
+      component.links[0].link.hasFocus,
+      'after moving down from first link, it DOES not have focus',
+    );
+    assert.ok(component.links[1].link.hasFocus, 'second link has focus');
+    await component.links[1].up();
+    assert.notOk(
+      component.links[1].link.hasFocus,
+      'after moving up from second link, it does not have focus',
+    );
+    assert.ok(component.links[0].link.hasFocus, 'first link has focus');
+
+    // wrap-around navigation from first to last menu item
+    await component.links[0].up();
+    assert.notOk(component.links[0].link.hasFocus);
+    assert.ok(component.links[1].link.hasFocus);
+    // wrap-around navigation from last to first menu item
+    await component.links[1].down();
+    assert.ok(component.links[0].link.hasFocus);
+    assert.notOk(component.links[1].link.hasFocus);
+
+    // close menu on escape, left, right, and tab keys.
+    await component.links[0].esc();
+    assert.strictEqual(component.links.length, 0);
+    await component.toggle.click();
+    assert.strictEqual(component.links.length, linkCount);
+    assert.ok(component.links[0].link.hasFocus);
+    await component.links[0].left();
+    assert.strictEqual(component.links.length, 0);
+    await component.toggle.click();
+    assert.strictEqual(component.links.length, linkCount);
+    assert.ok(component.links[0].link.hasFocus);
+    await component.links[0].right();
+    assert.strictEqual(component.links.length, 0);
     await component.toggle.click();
+    assert.strictEqual(component.links.length, linkCount);
+    assert.ok(component.links[0].link.hasFocus);
+    await component.links[0].tab();
     assert.strictEqual(component.links.length, 0);
   });
 });
diff --git a/packages/frontend/tests/pages/components/user-menu.js b/packages/frontend/tests/pages/components/user-menu.js
index cc948aa349..3b4bff3951 100644
--- a/packages/frontend/tests/pages/components/user-menu.js
+++ b/packages/frontend/tests/pages/components/user-menu.js
@@ -1,5 +1,5 @@
 import { create, collection, triggerable } from 'ember-cli-page-object';
-import linkToWithAction from 'frontend/tests/pages/components/link-to-with-action';
+import { hasFocus } from 'ilios-common';
 
 export default create({
   scope: '[data-test-user-menu]',
@@ -10,6 +10,17 @@ export default create({
     esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }),
   },
   links: collection('[data-test-item]', {
-    link: linkToWithAction,
+    hasFocus: hasFocus(),
+    link: {
+      scope: '[data-test-link-to-with-action]',
+      hasFocus: hasFocus(),
+    },
+    mouseEnter: triggerable('mouseenter'),
+    down: triggerable('keyup', '', { eventProperties: { key: 'ArrowDown' } }),
+    esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }),
+    left: triggerable('keyup', '', { eventProperties: { key: 'ArrowLeft' } }),
+    right: triggerable('keyup', '', { eventProperties: { key: 'ArrowRight' } }),
+    tab: triggerable('keyup', '', { eventProperties: { key: 'Tab' } }),
+    up: triggerable('keyup', '', { eventProperties: { key: 'ArrowUp' } }),
   }),
 });

From f259a9ab29cea686999bd2750ad0e359bed02d17 Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Mon, 7 Oct 2024 16:38:07 -0700
Subject: [PATCH 5/9] fixed some tests

---
 .../tests/pages/components/link-to-with-action.js         | 2 ++
 packages/frontend/tests/pages/components/user-menu.js     | 8 ++------
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/packages/frontend/tests/pages/components/link-to-with-action.js b/packages/frontend/tests/pages/components/link-to-with-action.js
index 5f8e367520..4fbd4a156c 100644
--- a/packages/frontend/tests/pages/components/link-to-with-action.js
+++ b/packages/frontend/tests/pages/components/link-to-with-action.js
@@ -1,8 +1,10 @@
 import { attribute, clickable, create, hasClass } from 'ember-cli-page-object';
+import { hasFocus } from 'ilios-common';
 
 const definition = {
   scope: '[data-test-link-to-with-action]',
   url: attribute('href'),
+  hasFocus: hasFocus(),
   isActive: hasClass('active'),
   click: clickable(),
 };
diff --git a/packages/frontend/tests/pages/components/user-menu.js b/packages/frontend/tests/pages/components/user-menu.js
index 3b4bff3951..3da2e8f6ba 100644
--- a/packages/frontend/tests/pages/components/user-menu.js
+++ b/packages/frontend/tests/pages/components/user-menu.js
@@ -1,5 +1,5 @@
 import { create, collection, triggerable } from 'ember-cli-page-object';
-import { hasFocus } from 'ilios-common';
+import linkToWithAction from 'frontend/tests/pages/components/link-to-with-action';
 
 export default create({
   scope: '[data-test-user-menu]',
@@ -10,11 +10,7 @@ export default create({
     esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }),
   },
   links: collection('[data-test-item]', {
-    hasFocus: hasFocus(),
-    link: {
-      scope: '[data-test-link-to-with-action]',
-      hasFocus: hasFocus(),
-    },
+    link: linkToWithAction,
     mouseEnter: triggerable('mouseenter'),
     down: triggerable('keyup', '', { eventProperties: { key: 'ArrowDown' } }),
     esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }),

From 3c79d2ffaa315ae8b93a9681d8aaedb62b88b413 Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Tue, 8 Oct 2024 07:53:00 -0700
Subject: [PATCH 6/9] changed setTimeout() to schedule('afterRender')

---
 packages/frontend/app/components/user-menu.js | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index a6befb936b..5a0eb608ef 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -1,4 +1,5 @@
 import Component from '@glimmer/component';
+import { schedule } from '@ember/runloop';
 import { service } from '@ember/service';
 import { cached, tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
@@ -21,9 +22,10 @@ export default class UserMenuComponent extends Component {
     this.isOpen = !this.isOpen;
 
     if (this.isOpen) {
-      setTimeout(() => {
+      // eslint-disable-next-line ember/no-runloop
+      schedule('afterRender', () => {
         document.querySelector('.user-menu .menu a:first-of-type').focus();
-      }, 1);
+      });
     }
   }
 
@@ -56,10 +58,10 @@ export default class UserMenuComponent extends Component {
   handleArrowDown(event, item) {
     if (event.target.tagName.toLowerCase() === 'button') {
       this.isOpen = true;
-      setTimeout(
-        () => event.target.parentElement.querySelector('.menu a:first-of-type').focus(),
-        1,
-      );
+      // eslint-disable-next-line ember/no-runloop
+      schedule('afterRender', () => {
+        event.target.parentElement.querySelector('.menu a:first-of-type').focus();
+      });
     } else {
       if (item.nextElementSibling) {
         item.nextElementSibling.querySelector('a').focus();

From 549fb7505cacd88560ca3a09a60777209f96162d Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Tue, 8 Oct 2024 08:31:55 -0700
Subject: [PATCH 7/9] reworked to avoid runloop and use a task instead

---
 packages/frontend/app/components/user-menu.js | 23 +++++++++----------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 5a0eb608ef..39ea6cdf2b 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -1,8 +1,8 @@
 import Component from '@glimmer/component';
-import { schedule } from '@ember/runloop';
 import { service } from '@ember/service';
 import { cached, tracked } from '@glimmer/tracking';
 import { action } from '@ember/object';
+import { task, timeout } from 'ember-concurrency';
 import { TrackedAsyncData } from 'ember-async-data';
 
 export default class UserMenuComponent extends Component {
@@ -17,15 +17,17 @@ export default class UserMenuComponent extends Component {
     return this.userModel.isResolved ? this.userModel.value : null;
   }
 
+  focusFirstLink = task(async () => {
+    await timeout(1);
+    document.querySelector('.user-menu .menu a:first-of-type').focus();
+  });
+
   @action
-  toggleMenu() {
+  async toggleMenu() {
     this.isOpen = !this.isOpen;
 
     if (this.isOpen) {
-      // eslint-disable-next-line ember/no-runloop
-      schedule('afterRender', () => {
-        document.querySelector('.user-menu .menu a:first-of-type').focus();
-      });
+      await this.focusFirstLink.perform();
     }
   }
 
@@ -55,13 +57,10 @@ export default class UserMenuComponent extends Component {
     return true;
   }
 
-  handleArrowDown(event, item) {
+  async handleArrowDown(event, item) {
     if (event.target.tagName.toLowerCase() === 'button') {
       this.isOpen = true;
-      // eslint-disable-next-line ember/no-runloop
-      schedule('afterRender', () => {
-        event.target.parentElement.querySelector('.menu a:first-of-type').focus();
-      });
+      await this.focusFirstLink.perform();
     } else {
       if (item.nextElementSibling) {
         item.nextElementSibling.querySelector('a').focus();
@@ -73,7 +72,7 @@ export default class UserMenuComponent extends Component {
 
   handleArrowUp(item) {
     if (item) {
-      if (item.previousElementSibling) {
+      if (item?.previousElementSibling) {
         item.previousElementSibling.querySelector('a').focus();
       } else {
         item.parentElement.lastElementChild.querySelector('a').focus();

From d9e11e46655079df654c5e0a8ecc013479accc78 Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michael.chadwick@gmail.com>
Date: Tue, 8 Oct 2024 08:32:28 -0700
Subject: [PATCH 8/9] made handleArrowUp not use extra item check

---
 packages/frontend/app/components/user-menu.js | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 39ea6cdf2b..48c0a5dcbd 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -71,12 +71,10 @@ export default class UserMenuComponent extends Component {
   }
 
   handleArrowUp(item) {
-    if (item) {
-      if (item?.previousElementSibling) {
-        item.previousElementSibling.querySelector('a').focus();
-      } else {
-        item.parentElement.lastElementChild.querySelector('a').focus();
-      }
+    if (item?.previousElementSibling) {
+      item.previousElementSibling.querySelector('a').focus();
+    } else {
+      item.parentElement.lastElementChild.querySelector('a').focus();
     }
   }
 }

From 13439c545f5f2ce2b3ade8644888cc3fdb2556cd Mon Sep 17 00:00:00 2001
From: Michael Chadwick <michaelchadwick@users.noreply.github.com>
Date: Tue, 8 Oct 2024 09:21:10 -0700
Subject: [PATCH 9/9] add optional chain check

Co-authored-by: Jon Johnson <jon.johnson@ucsf.edu>
---
 packages/frontend/app/components/user-menu.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/frontend/app/components/user-menu.js b/packages/frontend/app/components/user-menu.js
index 48c0a5dcbd..c66382eb89 100644
--- a/packages/frontend/app/components/user-menu.js
+++ b/packages/frontend/app/components/user-menu.js
@@ -74,7 +74,7 @@ export default class UserMenuComponent extends Component {
     if (item?.previousElementSibling) {
       item.previousElementSibling.querySelector('a').focus();
     } else {
-      item.parentElement.lastElementChild.querySelector('a').focus();
+      item?.parentElement.lastElementChild.querySelector('a').focus();
     }
   }
 }