Change focus management in disclosure utility component

.changeset/olive-parrots-work.md

@@ -0,0 +1,5 @@
+---
+"@hashicorp/design-system-components": patch
+---
+
+Change focus management in disclosure utility component

packages/components/addon/components/hds/disclosure/index.hbs

@@ -1,16 +1,16 @@
-<div class="hds-disclosure" ...attributes>
- <div class="hds-disclosure__toggle">
+{{! template-lint-disable no-invalid-interactive }}
+<div
+ class="hds-disclosure"
+ ...attributes
+ {{did-insert this.didInsert}}
+ {{on "focusout" this.onFocusOut}}
+ {{on "keyup" this.onKeyUp}}
+>
+ <div class="hds-disclosure__toggle" tabindex="-1">
  {{yield (hash onClickToggle=this.onClickToggle isActive=this.isActive) to="toggle"}}
  </div>
  {{#if this.isActive}}
- <div
- class="hds-disclosure__content"
- {{focus-trap
- isActive=this.isActive
- shouldSelfFocus=true
- focusTrapOptions=(hash clickOutsideDeactivates=this.clickOutsideDeactivates onDeactivate=this.onDeactivate)
- }}
- >
+ <div class="hds-disclosure__content" tabindex="-1">
  {{yield to="content"}}
  </div>
  {{/if}}

packages/components/addon/components/hds/disclosure/index.js

@@ -7,41 +7,48 @@ export default class HdsDisclosureComponent extends Component {
  @tracked toggleRef;
  @tracked isToggleClicked;
 
+ @action
+ didInsert(element) {
+ this.element = element;
+ }
+
  @action
  onClickToggle(event) {
  // we store a reference to the DOM node that has the "onClickToggle" event associated with it
  if (!this.toggleRef) {
  this.toggleRef = event.currentTarget;
  }
  this.isActive = !this.isActive;
+ // we explicitly apply a focus state to the toggle element to overcome a bug in WebKit (see b8abfcf)
+ this.toggleRef.focus();
  }
 
  @action
- clickOutsideDeactivates(event) {
- // we check if the toggle reference belongs to the tree of parent DOM nodes
- // of the element that was clicked and triggered the "click outside" event handling
- // notice: we use "event.composedPath" here because is now fully supported (see https://caniuse.com/?search=event%20path)
- const path = event.composedPath();
- this.isToggleClicked = path.includes(this.toggleRef);
- // here we need to return `true` to make sure that the focus trap will be deactivated (and allow the click event to do its thing (i.e. to pass-through to the element that was clicked).
- // see: https://github.com/focus-trap/focus-trap#createoptions
- return true;
+ onFocusOut(event) {
+ if (
+ !event.relatedTarget || // click or tap a non-related target (e.g. outside the element)
+ !this.element.contains(event.relatedTarget) // move focus to a target outside the element
+ ) {
+ this.deactivate();
+ }
  }
 
  @action
- onDeactivate() {
- // on deactivate we hide the content, except for the case when the button has been clicked
- // the reason is that the "onClickToggle" is called in any case (there's no way to block the event)
- // so when the user clicks the toggle to close the panel, we let the "onClickToggle" handle the closure
- // otherwise we would have two changes of status, this and the toggle, and the panel would remain open
- if (!this.isToggleClicked) {
+ onKeyUp(event) {
+ if (event.key === 'Escape') {
+ this.deactivate();
+ this.toggleRef.focus();
+ }
+ }
+
+ @action
+ deactivate() {
+ if (this.isActive) {
  this.isActive = false;
- // we need to reset this check
- this.isToggleClicked = false;
- // we call the "onClose" callback if it exists (and is a function)
- if (this.args.onClose && typeof this.args.onClose === 'function') {
- this.args.onClose();
- }
+ }
+ // we call the "onClose" callback if it exists (and is a function)
+ if (this.args.onClose && typeof this.args.onClose === 'function') {
+ this.args.onClose();
  }
  }
 }

packages/components/package.json

@@ -40,7 +40,6 @@
  "ember-cli-babel": "^7.26.11",
  "ember-cli-htmlbars": "^6.0.1",
  "ember-cli-sass": "^10.0.1",
- "ember-focus-trap": "^1.0.1",
  "ember-keyboard": "^8.1.0",
  "ember-named-blocks-polyfill": "^0.2.5",
  "sass": "^1.43.4"

packages/components/tests/dummy/app/templates/utilities/disclosure.hbs

@@ -85,25 +85,14 @@
  <code class="dummy-code">enter/return</code>), clicking outside of the content, or via the
  <code class="dummy-code">esc</code>
  key.</p>
- <p class="dummy-paragraph"><strong>Important:</strong></p>
- <ul>
- <li class="dummy-paragraph">
- the
- <code class="dummy-code">HDS::Disclosure</code>
- component will trap the focus inside the "content" while it's open (we're using the
- <a href="https://github.com/focus-trap/focus-trap" target="_blank" rel="noopener noreferrer">
- <code class="dummy-code">
- focus-trap</code>
- </a>
- library); at the moment we don't allow non-focusable content (it will trigger an error).
- </li>
- <li class="dummy-paragraph">the "content" is not positioned in any way in relation to the toggle: this
- responsibility is left to the consumers (eg by applying a
- <code class="dummy-code">position: absolute</code>
- to a wrapper around the content that is passed to the
- <code class="dummy-code">:content</code>
- block).</li>
- </ul>
+ <p class="dummy-paragraph">
+ <strong>Important:</strong>
+ The "content" is not positioned in any way in relation to the toggle: this responsibility is left to the consumers
+ (eg by applying a
+ <code class="dummy-code">position: absolute</code>
+ to a wrapper around the content that is passed to the
+ <code class="dummy-code">:content</code>
+ block).</p>
 </section>
 
 <section data-test-percy>
@@ -123,9 +112,7 @@
  </button>
  </:toggle>
  <:content>
- <a href="#">
- <DummyPlaceholder @text="some generic content here" @width="200" @height="90" @background="#FAFAFA" />
- </a>
+ <DummyPlaceholder @text="some generic content here" @width="200" @height="90" @background="#FAFAFA" />
  </:content>
  </Hds::Disclosure>
  </div>

packages/components/tests/integration/components/hds/disclosure/index-test.js

@@ -1,21 +1,14 @@
-import { module, test, skip } from 'qunit';
+import { module, test } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import {
  click,
+ triggerEvent,
  triggerKeyEvent,
- // waitFor,
  render,
  resetOnerror,
 } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 
-// we need to wait for the next event loop cycle after clicking the "toggle"
-// otherwise the test don't work (surely related to the focus-trap 'delayInitialFocus' parameter)
-// see https://github.com/focus-trap/focus-trap#createoptions
-// notice: you can use console.log(document.activeElement) to check which element has focus
-const waitNextExecutionFrame = () =>
- new Promise((resolve) => setTimeout(resolve, 100));
-
 module('Integration | Component | hds/disclosure/index', function (hooks) {
  setupRenderingTest(hooks);
 
@@ -59,11 +52,13 @@ module('Integration | Component | hds/disclosure/index', function (hooks) {
  assert.dom('.hds-disclosure__content').exists();
  assert.dom('a#test-disclosure-link').exists();
  });
- // TODO this doesn't work
- skip('it should render the "content" when the "toggle" is activated via "enter"', async function (assert) {
- assert.expect(2);
+
+ // ESCAPE KEY
+
+ test('it should hide the "content" when the "toggle" is deactivated via "Escape"', async function (assert) {
+ assert.expect(4);
  await render(hbs`
- <Hds::Disclosure>
+ <Hds::Disclosure id="test-disclosure">
  <:toggle as |t|>
  <button type="button" id="test-disclosure-button" {{on "click" t.onClickToggle}} />
  </:toggle>
@@ -72,42 +67,33 @@ module('Integration | Component | hds/disclosure/index', function (hooks) {
  </:content>
  </Hds::Disclosure>
  `);
- await triggerKeyEvent('button#test-disclosure-button', 'keydown', 'Enter');
- // await waitNextExecutionFrame();
- // await waitFor('.hds-disclosure__content', { timeout: 2000 });
+ await click('button#test-disclosure-button');
  assert.dom('.hds-disclosure__content').exists();
  assert.dom('a#test-disclosure-link').exists();
+ await triggerKeyEvent('#test-disclosure', 'keyup', 'Escape');
+ assert.dom('.hds-disclosure__content').doesNotExist();
+ assert.dom('a#test-disclosure-link').doesNotExist();
  });
 
- // FOCUS
+ // FOCUS OUT
 
- // TODO this doesn't work
- // see https://github.com/emberjs/ember-test-helpers/issues/738
- // https://discord.com/channels/480462759797063690/480523424121356298/842578755633545276
- // https://github.com/emberjs/ember-test-helpers/issues/626
- // https://discord.com/channels/480462759797063690/483601670685720591/831546103266148403
- skip('it should trap the focus inside the "content" block', async function (assert) {
- assert.expect(3);
+ test('it should hide the "content" when the focus is moved outside', async function (assert) {
+ assert.expect(4);
  await render(hbs`
- <Hds::Disclosure>
+ <Hds::Disclosure id="test-disclosure">
  <:toggle as |t|>
  <button type="button" id="test-disclosure-button" {{on "click" t.onClickToggle}} />
  </:toggle>
  <:content>
- <a id="test-disclosure-link-1" href="#">test1</a>
- <a id="test-disclosure-link-2" href="#">test2</a>
+ <a id="test-disclosure-link" href="#">test</a>
  </:content>
  </Hds::Disclosure>
  `);
  await click('button#test-disclosure-button');
- await waitNextExecutionFrame();
- // console.log('BBB', document.activeElement);
- assert.dom('a#test-disclosure-link-1').isFocused();
- await triggerKeyEvent('a#test-disclosure-link-1', 'keydown', 'Tab');
- // console.log('CCC', document.activeElement);
- assert.dom('a#test-disclosure-link-2').isFocused();
- await triggerKeyEvent('a#test-disclosure-link-2', 'keydown', 'Tab');
- // console.log('DDD', document.activeElement);
- assert.dom('a#test-disclosure-link-1').isFocused();
+ assert.dom('.hds-disclosure__content').exists();
+ assert.dom('a#test-disclosure-link').exists();
+ await triggerEvent('#test-disclosure', 'focusout');
+ assert.dom('.hds-disclosure__content').doesNotExist();
+ assert.dom('a#test-disclosure-link').doesNotExist();
  });
 });

yarn.lock

@@ -1383,7 +1383,7 @@
  ember-cli-version-checker "^5.1.2"
  semver "^7.3.5"
 
-"@embroider/addon-shim@^1.0.0", "@embroider/addon-shim@^1.5.0":
+"@embroider/addon-shim@^1.5.0":
  version "1.5.0"
  resolved "https://registry.yarnpkg.com/@embroider/addon-shim/-/addon-shim-1.5.0.tgz#639b8b394336a5ae26dd3e24ffc3d34d864ac5ce"
  integrity sha512-5zgwA/wTYjgn2Oo6hKRQhF/5Gnwb+hGhj/WXhZQa5yA7fRRdBV1tVMS7b7SLawZcmOhuWkyPwFdgsYtGBvDB0w==
@@ -6568,14 +6568,6 @@ ember-fetch@^8.1.1:
  node-fetch "^2.6.1"
  whatwg-fetch "^3.6.2"
 
-ember-focus-trap@^1.0.1:
- version "1.0.1"
- resolved "https://registry.yarnpkg.com/ember-focus-trap/-/ember-focus-trap-1.0.1.tgz#a99565f6ce55d500b92a0965e79e3ad04219f157"
- integrity sha512-ZUyq5ZkIuXp+ng9rCMkqBh36/V95PltL7iljStkma4+651xlAy3Z84L9WOu/uOJyVpNUxii8RJBbAySHV6c+RQ==
- dependencies:
- "@embroider/addon-shim" "^1.0.0"
- focus-trap "^6.7.1"
-
 ember-keyboard@^8.1.0:
  version "8.1.1"
  resolved "https://registry.yarnpkg.com/ember-keyboard/-/ember-keyboard-8.1.1.tgz#b12844a0f87cbb091b7c21293afccbf5aedc2ea3"
@@ -7879,13 +7871,6 @@ flush-write-stream@^1.0.0:
  inherits "^2.0.3"
  readable-stream "^2.3.6"
 
-focus-trap@^6.7.1:
- version "6.7.3"
- resolved "https://registry.yarnpkg.com/focus-trap/-/focus-trap-6.7.3.tgz#b5dc195b49c90001f08a63134471d1e6dd381ddd"
- integrity sha512-8xCEKndV4KrseGhFKKKmczVA14yx1/hnmFICPOjcFjToxCJYj/NHH43tPc3YE/PLnLRNZoFug0EcWkGQde/miQ==
- dependencies:
- tabbable "^5.2.1"
-
 follow-redirects@^1.0.0, follow-redirects@^1.14.0:
  version "1.14.9"
  resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.14.9.tgz#dd4ea157de7bfaf9ea9b3fbd85aa16951f78d8d7"
@@ -13260,11 +13245,6 @@ sync-disk-cache@^2.0.0:
  rimraf "^3.0.0"
  username-sync "^1.0.2"
 
-tabbable@^5.2.1:
- version "5.2.1"
- resolved "https://registry.yarnpkg.com/tabbable/-/tabbable-5.2.1.tgz#e3fda7367ddbb172dcda9f871c0fdb36d1c4cd9c"
- integrity sha512-40pEZ2mhjaZzK0BnI+QGNjJO8UYx9pP5v7BGe17SORTO0OEuuaAwQTkAp8whcZvqon44wKFOikD+Al11K3JICQ==
-
 table@^6.0.9:
  version "6.8.0"
  resolved "https://registry.yarnpkg.com/table/-/table-6.8.0.tgz#87e28f14fa4321c3377ba286f07b79b281a3b3ca"