Merge pull request #8179 from michaelchadwick/remove-user-menu-render…

…-modifier

Remove UserMenu component render modifier

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

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,17 +24,22 @@
     {{#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}}
               @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>

packages/frontend/app/components/user-menu.js

@@ -1,15 +1,14 @@
 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 {
   @service intl;
   @service currentUser;
   @tracked isOpen = false;
-  @tracked element;
 
   userModel = new TrackedAsyncData(this.currentUser.getModel());
 
@@ -18,9 +17,18 @@ 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) {
+      await this.focusFirstLink.perform();
+    }
   }
 
   @action
@@ -49,30 +57,24 @@ export default class UserMenuComponent extends Component {
     return true;
   }
 
-  handleArrowDown(evt, item) {
-    if (evt.target.tagName.toLowerCase() === 'button') {
+  async handleArrowDown(event, item) {
+    if (event.target.tagName.toLowerCase() === 'button') {
       this.isOpen = true;
+      await this.focusFirstLink.perform();
     } 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) {
+    if (item?.previousElementSibling) {
       item.previousElementSibling.querySelector('a').focus();
     } else {
-      this.element.querySelector('.menu li:last-of-type a').focus();
+      item?.parentElement.lastElementChild.querySelector('a').focus();
     }
   }
-
-  focus(el) {
-    el.focus();
-  }
 }

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,22 +35,22 @@ 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) {
     await render(hbs`<UserMenu />`);
 
     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);
   });
 });

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(),
 };

packages/frontend/tests/pages/components/user-menu.js

@@ -11,5 +11,12 @@ export default create({
   },
   links: collection('[data-test-item]', {
     link: linkToWithAction,
+    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' } }),
   }),
 });