Merge pull request #7922 from stopfstedt/locale-chooser

replace {{did-insert}} modifiers in LocaleChooser component

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

packages/frontend/app/components/instructor-groups/new.hbs

@@ -10,7 +10,7 @@
         </label>
         <input
           id="title-{{templateId}}"
-          {{autofocus}}
+          {{focus}}
           type="text"
           disabled={{this.save.isRunning}}
           placeholder={{t "general.instructorGroupTitlePlaceholder"}}
@@ -19,31 +19,21 @@
           {{on "keyup" this.keyboard}}
           {{on "keyup" (fn this.addErrorDisplayFor "title")}}
           {{on "input" (pick "target.value" (set this.title))}}
-        >
+        />
         <ValidationError @errors={{get-errors-for this.title}} />
       </div>
       <div class="buttons">
-        <button
-          type="button"
-          class="done text"
-          {{on "click" (perform this.save)}}
-          data-test-done
-        >
+        <button type="button" class="done text" {{on "click" (perform this.save)}} data-test-done>
           {{#if this.save.isRunning}}
             <LoadingSpinner />
           {{else}}
             {{t "general.done"}}
           {{/if}}
         </button>
-        <button
-          type="button"
-          class="cancel text"
-          {{on "click" @cancel}}
-          data-test-cancel
-        >
+        <button type="button" class="cancel text" {{on "click" @cancel}} data-test-cancel>
           {{t "general.cancel"}}
         </button>
       </div>
     </div>
   </div>
-{{/let}}
+{{/let}}

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}}
   >
     <FaIcon @icon="globe" />
     <span id="{{this.uniqueId}}-locale-chooser-title">
@@ -25,9 +24,8 @@
     <div
       class="menu"
       role="menu"
-      {{did-insert this.focusOnFirstItem}}
     >
-      {{#each this.locales as |loc|}}
+      {{#each this.locales as |loc index|}}
         <button
           type="button"
           role="menuitemradio"
@@ -39,6 +37,7 @@
           {{on "click" (fn this.changeLocale loc.id)}}
           {{on "keyup" this.moveFocus}}
           {{on "mouseenter" this.clearFocus}}
+          {{focus (eq index 0)}}
         >
           {{loc.text}}
         </button>

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');
@@ -26,28 +25,17 @@ export default class LocaleChooserComponent extends Component {
     return guidFor(this);
   }
 
-  @action
-  focusOnFirstItem(menuElement) {
-    this.menuElement = menuElement;
-    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

packages/frontend/tests/integration/components/locale-chooser-test.js

@@ -57,8 +57,19 @@ module('Integration | Component | locale-chooser', function (hooks) {
     await render(hbs`<LocaleChooser />`);
 
     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);
+  });
+
+  test('first item in menu receives focus when menu is opened', async function (assert) {
+    await render(hbs`<LocaleChooser />`);
+    await component.toggle.click();
+    assert.strictEqual(component.locales.length, 3);
+    assert.ok(component.locales[0].hasFocus);
+    assert.notOk(component.locales[1].hasFocus);
+    assert.notOk(component.locales[2].hasFocus);
   });
 });

packages/frontend/tests/pages/components/locale-chooser.js

@@ -1,12 +1,15 @@
 import { create, collection, triggerable } from 'ember-cli-page-object';
-
+import { hasFocus } from 'ilios-common';
 export default create({
   scope: '[data-test-locale-chooser]',
   toggle: {
     scope: '[data-test-toggle]',
     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]', {}),
+  locales: collection('[data-test-item]', {
+    hasFocus: hasFocus(),
+  }),
 });

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,
 

packages/ilios-common/addon/modifiers/focus.js

@@ -0,0 +1,23 @@
+import { modifier } from 'ember-modifier';
+
+/**
+  The `{{focus}}` element modifier sets the focus its DOM element.
+  It can take one optional boolean argument, which can be used to conditionally allow or prevent focusing.
+
+  For example - focusing the first item in an ordered list:
+
+  ```handlebars
+  <ol>
+  {{#each @items as |item index|}}
+    <li {{focus (eq index 0)}}>
+  {{/each}
+  </ol>
+  ```
+*/
+export default modifier(function focus(element, positional /*, named*/) {
+  // If no argument is provided, then the given element is focused on.
+  // If an argument has been provided, then only set focus if that arg is truthy.
+  if (!positional.length || !!positional[0]) {
+    element.focus();
+  }
+});

packages/ilios-common/app/modifiers/focus.js

@@ -0,0 +1 @@
+export { default } from 'ilios-common/modifiers/focus';

packages/test-app/tests/integration/modifiers/focus-test.js

@@ -0,0 +1,29 @@
+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 | focus', function (hooks) {
+  setupRenderingTest(hooks);
+
+  test('it focuses by default without condition', async function (assert) {
+    this.set('label', 'foo bar');
+    await render(hbs`<label><input {{focus}} />{{this.label}}</label>
+`);
+    assert.dom('input').isFocused();
+  });
+
+  test('it focuses when condition is met', async function (assert) {
+    this.set('label', 'foo bar');
+    await render(hbs`<label><input {{focus true}} />{{this.label}}</label>
+`);
+    assert.dom('input').isFocused();
+  });
+
+  test('it does not focus when condition is not met', async function (assert) {
+    this.set('label', 'foo bar');
+    await render(hbs`<label><input {{focus false}} />{{this.label}}</label>
+`);
+    assert.dom('input').isNotFocused();
+  });
+});