Skip to content

Commit a5d8983

Browse files
dartajaxdartajax
authored
Merge pull request #8179 from michaelchadwick/remove-user-menu-render-modifier
Remove UserMenu component render modifier
2 parents fee5dba + 13439c5 commit a5d8983

File tree

6 files changed

+95
-25
lines changed

6 files changed

+95
-25
lines changed

‎packages/frontend/.lint-todo‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1722902400000|1730682000000|1754006400000|app/components/new-directory-user.hbs
2-
add|ember-template-lint|no-at-ember-render-modifiers|28|43|28|43|5c127b0b8124a93b18177d7580bcc47dbb8ebbff|1722902400000|1730682000000|1754006400000|app/components/user-menu.hbs
3-
add|ember-template-lint|no-at-ember-render-modifiers|5|4|5|4|1459921678f69a3a659ce69b1da303bc8e96b104|1725580800000|1728172800000|1730768400000|app/components/user-menu.hbs
42
add|ember-template-lint|no-at-ember-render-modifiers|4|2|4|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1722902400000|1730682000000|1754006400000|app/components/user-profile-bio.hbs
53
add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|f53982efe02d2bef9e7f12b5b862288c594579c2|1722902400000|1730682000000|1754006400000|app/components/user-profile-bio.hbs
64
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‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
<nav
33
class="user-menu{{if this.isOpen ' is-open'}}"
44
aria-label={{t "general.userMenu"}}
5-
{{did-insert (set this "element")}}
65
{{! template-lint-disable no-invalid-interactive }}
76
{{on "keyup" this.keyUp}}
87
data-test-user-menu
@@ -25,17 +24,22 @@
2524
{{#if this.isOpen}}
2625
<div {{on-click-outside (set this "isOpen" false)}}>
2726
<ul class="menu">
28-
<li tabindex="-1" data-test-item {{did-insert this.focus}}>
27+
<li tabindex="-1" data-test-item {{focus}}>
2928
<LinkToWithAction
3029
@route="myprofile"
3130
@action={{set this "isOpen" false}}
3231
@queryParams={{hash invalidatetokens=null newtoken=null}}
32+
data-test-item-link
3333
>
3434
{{t "general.myProfile"}}
3535
</LinkToWithAction>
3636
</li>
3737
<li tabindex="-1" data-test-item>
38-
<LinkToWithAction @route="logout" @action={{set this "isOpen" false}}>
38+
<LinkToWithAction
39+
@route="logout"
40+
@action={{set this "isOpen" false}}
41+
data-test-item-link
42+
>
3943
{{t "general.logout"}}
4044
</LinkToWithAction>
4145
</li>

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import Component from '@glimmer/component';
2-
import { schedule } from '@ember/runloop';
32
import { service } from '@ember/service';
43
import { cached, tracked } from '@glimmer/tracking';
54
import { action } from '@ember/object';
5+
import { task, timeout } from 'ember-concurrency';
66
import { TrackedAsyncData } from 'ember-async-data';
77

88
export default class UserMenuComponent extends Component {
99
@service intl;
1010
@service currentUser;
1111
@tracked isOpen = false;
12-
@tracked element;
1312

1413
userModel = new TrackedAsyncData(this.currentUser.getModel());
1514

@@ -18,9 +17,18 @@ export default class UserMenuComponent extends Component {
1817
return this.userModel.isResolved ? this.userModel.value : null;
1918
}
2019

20+
focusFirstLink = task(async () => {
21+
await timeout(1);
22+
document.querySelector('.user-menu .menu a:first-of-type').focus();
23+
});
24+
2125
@action
22-
toggleMenu() {
26+
async toggleMenu() {
2327
this.isOpen = !this.isOpen;
28+
29+
if (this.isOpen) {
30+
await this.focusFirstLink.perform();
31+
}
2432
}
2533

2634
@action
@@ -49,30 +57,24 @@ export default class UserMenuComponent extends Component {
4957
return true;
5058
}
5159

52-
handleArrowDown(evt, item) {
53-
if (evt.target.tagName.toLowerCase() === 'button') {
60+
async handleArrowDown(event, item) {
61+
if (event.target.tagName.toLowerCase() === 'button') {
5462
this.isOpen = true;
63+
await this.focusFirstLink.perform();
5564
} else {
5665
if (item.nextElementSibling) {
5766
item.nextElementSibling.querySelector('a').focus();
5867
} else {
59-
// eslint-disable-next-line ember/no-runloop
60-
schedule('afterRender', () => {
61-
this.element.querySelector('.menu li:nth-of-type(1) a').focus();
62-
});
68+
item.parentElement.firstElementChild.querySelector('a').focus();
6369
}
6470
}
6571
}
6672

6773
handleArrowUp(item) {
68-
if (item.previousElementSibling) {
74+
if (item?.previousElementSibling) {
6975
item.previousElementSibling.querySelector('a').focus();
7076
} else {
71-
this.element.querySelector('.menu li:last-of-type a').focus();
77+
item?.parentElement.lastElementChild.querySelector('a').focus();
7278
}
7379
}
74-
75-
focus(el) {
76-
el.focus();
77-
}
7880
}

‎packages/frontend/tests/integration/components/user-menu-test.js‎

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { module, test } from 'qunit';
22
import { setupRenderingTest } from 'frontend/tests/helpers';
3-
import { render } from '@ember/test-helpers';
3+
import { render, waitFor } from '@ember/test-helpers';
44
import { hbs } from 'ember-cli-htmlbars';
55
import component from 'frontend/tests/pages/components/user-menu';
66
import a11yAudit from 'ember-a11y-testing/test-support/audit';
@@ -11,6 +11,9 @@ module('Integration | Component | user-menu', function (hooks) {
1111
setupRenderingTest(hooks);
1212
setupMirage(hooks);
1313

14+
// My Profile and Logout
15+
const linkCount = 2;
16+
1417
hooks.beforeEach(async function () {
1518
await setupAuthentication();
1619
});
@@ -32,22 +35,22 @@ module('Integration | Component | user-menu', function (hooks) {
3235

3336
assert.strictEqual(component.links.length, 0);
3437
await component.toggle.click();
35-
assert.strictEqual(component.links.length, 2);
38+
assert.strictEqual(component.links.length, linkCount);
3639
});
3740

3841
test('down opens menu', async function (assert) {
3942
await render(hbs`<UserMenu />`);
4043

4144
assert.strictEqual(component.links.length, 0);
4245
await component.toggle.down();
43-
assert.strictEqual(component.links.length, 2);
46+
assert.strictEqual(component.links.length, linkCount);
4447
});
4548

4649
test('escape closes menu', async function (assert) {
4750
await render(hbs`<UserMenu />`);
4851

4952
await component.toggle.down();
50-
assert.strictEqual(component.links.length, 2);
53+
assert.strictEqual(component.links.length, linkCount);
5154
await component.toggle.esc();
5255
assert.strictEqual(component.links.length, 0);
5356
});
@@ -56,8 +59,62 @@ module('Integration | Component | user-menu', function (hooks) {
5659
await render(hbs`<UserMenu />`);
5760

5861
await component.toggle.down();
59-
assert.strictEqual(component.links.length, 2);
62+
assert.strictEqual(component.links.length, linkCount);
63+
await component.toggle.click();
64+
assert.strictEqual(component.links.length, 0);
65+
});
66+
67+
test('keyboard navigation', async function (assert) {
68+
await render(hbs`<UserMenu />`);
69+
await component.toggle.click();
70+
assert.strictEqual(component.links.length, linkCount, `has ${linkCount} links`);
71+
72+
// slight delay to allow for proper loading of popup menu
73+
await waitFor('.user-menu .menu');
74+
75+
assert.ok(component.links[0].link.hasFocus, 'first link has focus');
76+
assert.notOk(component.links[1].link.hasFocus, 'second link does NOT have focus');
77+
78+
// regular down/up navigation
79+
await component.links[0].down();
80+
assert.notOk(
81+
component.links[0].link.hasFocus,
82+
'after moving down from first link, it DOES not have focus',
83+
);
84+
assert.ok(component.links[1].link.hasFocus, 'second link has focus');
85+
await component.links[1].up();
86+
assert.notOk(
87+
component.links[1].link.hasFocus,
88+
'after moving up from second link, it does not have focus',
89+
);
90+
assert.ok(component.links[0].link.hasFocus, 'first link has focus');
91+
92+
// wrap-around navigation from first to last menu item
93+
await component.links[0].up();
94+
assert.notOk(component.links[0].link.hasFocus);
95+
assert.ok(component.links[1].link.hasFocus);
96+
// wrap-around navigation from last to first menu item
97+
await component.links[1].down();
98+
assert.ok(component.links[0].link.hasFocus);
99+
assert.notOk(component.links[1].link.hasFocus);
100+
101+
// close menu on escape, left, right, and tab keys.
102+
await component.links[0].esc();
103+
assert.strictEqual(component.links.length, 0);
104+
await component.toggle.click();
105+
assert.strictEqual(component.links.length, linkCount);
106+
assert.ok(component.links[0].link.hasFocus);
107+
await component.links[0].left();
108+
assert.strictEqual(component.links.length, 0);
109+
await component.toggle.click();
110+
assert.strictEqual(component.links.length, linkCount);
111+
assert.ok(component.links[0].link.hasFocus);
112+
await component.links[0].right();
113+
assert.strictEqual(component.links.length, 0);
60114
await component.toggle.click();
115+
assert.strictEqual(component.links.length, linkCount);
116+
assert.ok(component.links[0].link.hasFocus);
117+
await component.links[0].tab();
61118
assert.strictEqual(component.links.length, 0);
62119
});
63120
});

‎packages/frontend/tests/pages/components/link-to-with-action.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { attribute, clickable, create, hasClass } from 'ember-cli-page-object';
2+
import { hasFocus } from 'ilios-common';
23

34
const definition = {
45
scope: '[data-test-link-to-with-action]',
56
url: attribute('href'),
7+
hasFocus: hasFocus(),
68
isActive: hasClass('active'),
79
click: clickable(),
810
};

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,12 @@ export default create({
1111
},
1212
links: collection('[data-test-item]', {
1313
link: linkToWithAction,
14+
mouseEnter: triggerable('mouseenter'),
15+
down: triggerable('keyup', '', { eventProperties: { key: 'ArrowDown' } }),
16+
esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }),
17+
left: triggerable('keyup', '', { eventProperties: { key: 'ArrowLeft' } }),
18+
right: triggerable('keyup', '', { eventProperties: { key: 'ArrowRight' } }),
19+
tab: triggerable('keyup', '', { eventProperties: { key: 'Tab' } }),
20+
up: triggerable('keyup', '', { eventProperties: { key: 'ArrowUp' } }),
1421
}),
1522
});

0 commit comments

Comments
 (0)