From a470604e4241722551202ef9a52214665b4de150 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Tue, 2 Jul 2024 13:28:17 -0700 Subject: [PATCH 1/6] focus on locale picker toggle button on locale change without tracking that element. --- packages/frontend/app/components/locale-chooser.hbs | 1 - packages/frontend/app/components/locale-chooser.js | 10 ++-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/frontend/app/components/locale-chooser.hbs b/packages/frontend/app/components/locale-chooser.hbs index b0d86a1fe3..44404673e0 100644 --- a/packages/frontend/app/components/locale-chooser.hbs +++ b/packages/frontend/app/components/locale-chooser.hbs @@ -13,7 +13,6 @@ data-test-toggle {{on "click" (toggle "isOpen" this)}} {{on "keyup" this.toggleMenu}} - {{did-insert this.setMenuButton}} > diff --git a/packages/frontend/app/components/locale-chooser.js b/packages/frontend/app/components/locale-chooser.js index b336cead5a..0e98e31c30 100644 --- a/packages/frontend/app/components/locale-chooser.js +++ b/packages/frontend/app/components/locale-chooser.js @@ -9,7 +9,6 @@ export default class LocaleChooserComponent extends Component { @service intl; @tracked isOpen = false; @tracked menuElement; - @tracked menuButtonElement; get locale() { const locale = this.intl.get('primaryLocale'); @@ -32,22 +31,17 @@ export default class LocaleChooserComponent extends Component { menuElement.querySelector('button:first-of-type').focus(); } - @action - setMenuButton(menuButtonElement) { - this.menuButtonElement = menuButtonElement; - } - @action close() { this.isOpen = false; } @action - changeLocale(id) { + changeLocale(id, event) { this.isOpen = false; this.intl.setLocale(id); window.document.querySelector('html').setAttribute('lang', id); - this.menuButtonElement.focus(); + event.target.parentElement.parentElement.firstElementChild.focus(); } @action From d57d969b166298dd6a8d0bfebefa8fa4521a4fb8 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Tue, 2 Jul 2024 13:50:06 -0700 Subject: [PATCH 2/6] make selector input optional on has-focus helper. sometimes, the given element is all we need. --- .../addon-test-support/ilios-common/helpers/has-focus.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ilios-common/addon-test-support/ilios-common/helpers/has-focus.js b/packages/ilios-common/addon-test-support/ilios-common/helpers/has-focus.js index cfd50528e4..78b728e6b0 100644 --- a/packages/ilios-common/addon-test-support/ilios-common/helpers/has-focus.js +++ b/packages/ilios-common/addon-test-support/ilios-common/helpers/has-focus.js @@ -1,6 +1,6 @@ import { findOne } from 'ember-cli-page-object/extend'; -export function hasFocus(selector, userOptions = {}) { +export function hasFocus(selector = null, userOptions = {}) { return { isDescriptor: true, From 5ee8c9776193773fd68d4e7bb365935dacd7e2b9 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Tue, 2 Jul 2024 13:52:17 -0700 Subject: [PATCH 3/6] focus the toggle button on locale-change without the need to track that element. we have full control over this component's markup, so let's traverse it to markup from the click target to the toggle button, instead of tracking the toggle button separately. one less modifier. --- packages/frontend/.lint-todo | 1 + .../tests/integration/components/locale-chooser-test.js | 2 ++ packages/frontend/tests/pages/components/locale-chooser.js | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/frontend/.lint-todo b/packages/frontend/.lint-todo index c439e2bc8b..d0d6293820 100644 --- a/packages/frontend/.lint-todo +++ b/packages/frontend/.lint-todo @@ -98,3 +98,4 @@ add|ember-template-lint|no-at-ember-render-modifiers|10|6|10|6|d919d2af254f782c0 add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060b0e5d3f05baea24ba178878|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/session-type.hbs add|ember-template-lint|no-at-ember-render-modifiers|10|6|10|6|d919d2af254f782c01fe2ba15416673e52e91124|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060b0e5d3f05baea24ba178878|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs +remove|ember-template-lint|no-at-ember-render-modifiers|16|4|16|4|46d995f60904f6b6f4b1b6b24bb65b9c97441bc1|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs diff --git a/packages/frontend/tests/integration/components/locale-chooser-test.js b/packages/frontend/tests/integration/components/locale-chooser-test.js index f29f7993dc..18fedb3393 100644 --- a/packages/frontend/tests/integration/components/locale-chooser-test.js +++ b/packages/frontend/tests/integration/components/locale-chooser-test.js @@ -57,8 +57,10 @@ module('Integration | Component | locale-chooser', function (hooks) { await render(hbs``); await component.toggle.click(); + assert.notOk(component.toggle.hasFocus); await component.locales[1].click(); assert.strictEqual(component.locales.length, 0); assert.strictEqual(component.text, 'Español (es)'); + assert.ok(component.toggle.hasFocus); }); }); diff --git a/packages/frontend/tests/pages/components/locale-chooser.js b/packages/frontend/tests/pages/components/locale-chooser.js index 635d473936..dccb443fc1 100644 --- a/packages/frontend/tests/pages/components/locale-chooser.js +++ b/packages/frontend/tests/pages/components/locale-chooser.js @@ -1,5 +1,5 @@ import { create, collection, triggerable } from 'ember-cli-page-object'; - +import { hasFocus } from 'ilios-common'; export default create({ scope: '[data-test-locale-chooser]', toggle: { @@ -7,6 +7,7 @@ export default create({ enter: triggerable('keyup', '', { eventProperties: { key: 'Enter' } }), down: triggerable('keyup', '', { eventProperties: { key: 'ArrowDown' } }), esc: triggerable('keyup', '', { eventProperties: { key: 'Escape' } }), + hasFocus: hasFocus(), }, locales: collection('[data-test-item]', {}), }); From c7220ff99684a12aa2eebda2abbe78258cee2581 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Tue, 2 Jul 2024 15:47:04 -0700 Subject: [PATCH 4/6] replace did-insert modifier with focus modifier. drop-in replacement. --- packages/frontend/.lint-todo | 1 + .../app/components/locale-chooser.hbs | 4 +-- .../frontend/app/components/locale-chooser.js | 6 ---- .../components/locale-chooser-test.js | 9 ++++++ .../tests/pages/components/locale-chooser.js | 4 ++- .../ilios-common/addon/modifiers/focus.js | 23 +++++++++++++++ packages/ilios-common/app/modifiers/focus.js | 1 + .../tests/integration/modifiers/focus-test.js | 29 +++++++++++++++++++ 8 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 packages/ilios-common/addon/modifiers/focus.js create mode 100644 packages/ilios-common/app/modifiers/focus.js create mode 100644 packages/test-app/tests/integration/modifiers/focus-test.js diff --git a/packages/frontend/.lint-todo b/packages/frontend/.lint-todo index d0d6293820..04181e1edd 100644 --- a/packages/frontend/.lint-todo +++ b/packages/frontend/.lint-todo @@ -99,3 +99,4 @@ add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060 add|ember-template-lint|no-at-ember-render-modifiers|10|6|10|6|d919d2af254f782c01fe2ba15416673e52e91124|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060b0e5d3f05baea24ba178878|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs remove|ember-template-lint|no-at-ember-render-modifiers|16|4|16|4|46d995f60904f6b6f4b1b6b24bb65b9c97441bc1|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs +remove|ember-template-lint|no-at-ember-render-modifiers|28|6|28|6|df94e6558ff62dea69f6f656f668f29b56bcc378|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs diff --git a/packages/frontend/app/components/locale-chooser.hbs b/packages/frontend/app/components/locale-chooser.hbs index 44404673e0..883e7c633d 100644 --- a/packages/frontend/app/components/locale-chooser.hbs +++ b/packages/frontend/app/components/locale-chooser.hbs @@ -24,9 +24,8 @@
- -
-{{/let}} +{{/let}} \ No newline at end of file diff --git a/packages/ilios-common/addon/modifiers/autofocus.js b/packages/ilios-common/addon/modifiers/autofocus.js deleted file mode 100644 index 74cb1bd392..0000000000 --- a/packages/ilios-common/addon/modifiers/autofocus.js +++ /dev/null @@ -1,3 +0,0 @@ -import { modifier } from 'ember-modifier'; - -export default modifier((element) => element.focus(), { eager: false }); diff --git a/packages/ilios-common/app/modifiers/autofocus.js b/packages/ilios-common/app/modifiers/autofocus.js deleted file mode 100644 index d65ed34bf8..0000000000 --- a/packages/ilios-common/app/modifiers/autofocus.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from 'ilios-common/modifiers/autofocus'; diff --git a/packages/test-app/tests/integration/modifiers/autofocus-test.js b/packages/test-app/tests/integration/modifiers/autofocus-test.js deleted file mode 100644 index 3ebc9ac0d4..0000000000 --- a/packages/test-app/tests/integration/modifiers/autofocus-test.js +++ /dev/null @@ -1,16 +0,0 @@ -import { module, test } from 'qunit'; -import { setupRenderingTest } from 'test-app/tests/helpers'; -import { render } from '@ember/test-helpers'; -import { hbs } from 'ember-cli-htmlbars'; - -module('Integration | Modifier | autofocus', function (hooks) { - setupRenderingTest(hooks); - - test('it focuses', async function (assert) { - this.set('label', 'foo bar'); - await render(hbs` -`); - - assert.dom('input').isFocused(); - }); -}); From 396a0f136c7f32f260cea73509510439be52dbc6 Mon Sep 17 00:00:00 2001 From: Stefan Topfstedt Date: Mon, 8 Jul 2024 15:26:49 -0700 Subject: [PATCH 6/6] rm obsolete todos. --- packages/frontend/.lint-todo | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/frontend/.lint-todo b/packages/frontend/.lint-todo index 04181e1edd..d8e55f85d5 100644 --- a/packages/frontend/.lint-todo +++ b/packages/frontend/.lint-todo @@ -1,6 +1,3 @@ - -add|ember-template-lint|no-at-ember-render-modifiers|16|4|16|4|46d995f60904f6b6f4b1b6b24bb65b9c97441bc1|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs -add|ember-template-lint|no-at-ember-render-modifiers|28|6|28|6|df94e6558ff62dea69f6f656f668f29b56bcc378|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1719360000000|1734915600000|1750464000000|app/components/new-directory-user.hbs add|ember-template-lint|no-at-ember-render-modifiers|4|2|4|2|23cd787c79c34a628dadb6e96dd4004d42eebb79|1719360000000|1734915600000|1750464000000|app/components/school-competencies-collapsed.hbs add|ember-template-lint|no-at-ember-render-modifiers|5|2|5|2|d0274adc777d08699935bd8c5f7a4a3346d93065|1719360000000|1734915600000|1750464000000|app/components/school-competencies-collapsed.hbs @@ -98,5 +95,3 @@ add|ember-template-lint|no-at-ember-render-modifiers|10|6|10|6|d919d2af254f782c0 add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060b0e5d3f05baea24ba178878|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/session-type.hbs add|ember-template-lint|no-at-ember-render-modifiers|10|6|10|6|d919d2af254f782c01fe2ba15416673e52e91124|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs add|ember-template-lint|no-at-ember-render-modifiers|11|6|11|6|940005188b476a060b0e5d3f05baea24ba178878|1719360000000|1734915600000|1750464000000|app/components/reports/subject/new/term.hbs -remove|ember-template-lint|no-at-ember-render-modifiers|16|4|16|4|46d995f60904f6b6f4b1b6b24bb65b9c97441bc1|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs -remove|ember-template-lint|no-at-ember-render-modifiers|28|6|28|6|df94e6558ff62dea69f6f656f668f29b56bcc378|1719360000000|1734915600000|1750464000000|app/components/locale-chooser.hbs