From 5405641c1545072af1d2c69901b3200bc7071d7e Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 13:03:37 -0400 Subject: [PATCH 01/17] fix: dont lock scrolling on desktop --- .../src/components/hds/app-side-nav/index.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/components/src/components/hds/app-side-nav/index.ts b/packages/components/src/components/hds/app-side-nav/index.ts index e622306ade..9149bf3f63 100644 --- a/packages/components/src/components/hds/app-side-nav/index.ts +++ b/packages/components/src/components/hds/app-side-nav/index.ts @@ -175,10 +175,12 @@ export default class HdsAppSideNav extends Component { onToggleMinimizedStatus(this._isMinimized); } - if (this._isMinimized) { - this.unlockBodyScroll(); - } else { - this.lockBodyScroll(); + if (!this._isDesktop) { + if (this._isMinimized) { + this.unlockBodyScroll(); + } else { + this.lockBodyScroll(); + } } } @@ -215,6 +217,11 @@ export default class HdsAppSideNav extends Component { this.synchronizeInert(); + if (this._isDesktop) { + // make sure scrolling is enabled if the user resizes the window from mobile to desktop + this.unlockBodyScroll(); + } + const { onDesktopViewportChange } = this.args; if (typeof onDesktopViewportChange === 'function') { From f99562eff7f24f6e97aa4d8d4406658c47b6cc11 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 13:05:06 -0400 Subject: [PATCH 02/17] chore: changeset --- .changeset/honest-experts-battle.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/honest-experts-battle.md diff --git a/.changeset/honest-experts-battle.md b/.changeset/honest-experts-battle.md new file mode 100644 index 0000000000..3e42ee2c56 --- /dev/null +++ b/.changeset/honest-experts-battle.md @@ -0,0 +1,5 @@ +--- +"@hashicorp/design-system-components": patch +--- + +`AppSideNav` - fixed bug where scrolling was blocked when the `AppSideNav` was expanded on desktop views. From 696be6f6bfa5388da5213e978aa47f6442e2a24a Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 13:54:46 -0400 Subject: [PATCH 03/17] fix: re-enable body test for Modal --- .../tests/integration/components/hds/modal/index-test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/showcase/tests/integration/components/hds/modal/index-test.js b/showcase/tests/integration/components/hds/modal/index-test.js index 99d863b566..660e0571d3 100644 --- a/showcase/tests/integration/components/hds/modal/index-test.js +++ b/showcase/tests/integration/components/hds/modal/index-test.js @@ -3,7 +3,7 @@ * SPDX-License-Identifier: MPL-2.0 */ -import { module, test, skip } from 'qunit'; +import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; import { click, @@ -231,8 +231,7 @@ module('Integration | Component | hds/modal/index', function (hooks) { assert.dom('#test-button').isFocused(); }); - // not sure how to reach the `body` element, it says "body is not a valid root element" - skip('it returns focus to the `body` element, if the one that initiated the open event not anymore in the DOM', async function (assert) { + test('it returns focus to the `body` element, if the one that initiated the open event not anymore in the DOM', async function (assert) { await render( hbs` @@ -249,7 +248,7 @@ module('Integration | Component | hds/modal/index', function (hooks) { await click('#test-interactive'); assert.true(this.showModal); await click('button.hds-modal__dismiss'); - assert.dom('body', 'body').isFocused(); + assert.dom('body', document).isFocused(); }); test('it returns focus to a specific element if provided via`@returnFocusTo`', async function (assert) { From d5c7db53958f3734f80b2f33b1a87c87fd0762cf Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 14:19:14 -0400 Subject: [PATCH 04/17] fix: prevent users from tabbing to hidden links if dont use portal --- packages/components/src/components/hds/app-side-nav/index.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/components/hds/app-side-nav/index.hbs b/packages/components/src/components/hds/app-side-nav/index.hbs index bf6eba6c25..9d4cc9d2a7 100644 --- a/packages/components/src/components/hds/app-side-nav/index.hbs +++ b/packages/components/src/components/hds/app-side-nav/index.hbs @@ -27,7 +27,7 @@ /> {{/if}} -
+
{{~yield~}}
From 2df7b3e4b1e9400d80c2267f0edcfd3424230116 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 14:19:34 -0400 Subject: [PATCH 05/17] chore: add tests for the bugs fixed --- .../components/hds/app-side-nav/index-test.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index b5257c3e44..2438544bb9 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -11,6 +11,7 @@ import { resetOnerror, settled, triggerKeyEvent, + tab, } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; @@ -137,13 +138,19 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { hbs`` ); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); + assert.dom('body', document).doesNotHaveStyle('overflow'); await click('.hds-app-side-nav__toggle-button'); assert .dom('#test-app-side-nav') .hasClass('hds-app-side-nav--is-not-minimized'); + assert.dom('body', document).hasStyle({ + overflow: 'hidden', + }); + await click('.hds-app-side-nav__toggle-button'); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); + assert.dom('body', document).doesNotHaveStyle('overflow'); }); test('it collapses when the ESC key is pressed on narrow viewports', async function (assert) { @@ -199,6 +206,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { .dom('.hds-app-side-nav-hide-when-minimized') .doesNotHaveAttribute('inert'); assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); + assert.dom('body', document).doesNotHaveStyle('overflow'); await click('.hds-app-side-nav__toggle-button'); @@ -211,6 +219,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { .hasClass('hds-icon-chevrons-right'); assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); + assert.dom('body', document).doesNotHaveStyle('overflow'); }); test('when the viewport changes from desktop to mobile, it automatically collapses and becomes inert', async function (assert) { @@ -277,6 +286,62 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { assert .dom('.hds-app-side-nav-hide-when-minimized') .doesNotHaveAttribute('inert'); + assert.dom('body', document).doesNotHaveStyle('overflow'); + }); + + test('when collapsed and the viewport changes from mobile to desktop and is expanded, scrolling is enabled', async function (assert) { + this.mockMedia(); + + let calls = []; + this.setProperties({ + onDesktopViewportChange: (...args) => calls.push(args), + }); + + await render(hbs` + + +`); + + assert.dom('body', document).hasStyle({ + overflow: 'hidden', + }); + + await this.changeBrowserSize(false); + assert.deepEqual( + calls[1], + [false], + 'resizing to mobile triggers a false event' + ); + + await this.changeBrowserSize(true); + assert.deepEqual( + calls[2], + [true], + 'resizing to desktop triggers a true event' + ); + + assert.dom('body', document).doesNotHaveStyle('overflow'); + }); + + test('when collapsed, the content in the AppSideNav is not focusable', async function (assert) { + await render(hbs` + + +`); + + await click('.hds-app-side-nav__toggle-button'); + assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); + assert.dom('.hds-app-side-nav__toggle-button').isFocused(); + + await tab(); + assert.dom('#button-2').isFocused(); }); // CALLBACKS From c6e5edd7e5c7e652a3d115304e46d34e26bd4b6d Mon Sep 17 00:00:00 2001 From: shleewhite Date: Mon, 12 May 2025 14:26:58 -0400 Subject: [PATCH 06/17] chore: update changeset --- .changeset/honest-experts-battle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/honest-experts-battle.md b/.changeset/honest-experts-battle.md index 3e42ee2c56..c5a78e7fea 100644 --- a/.changeset/honest-experts-battle.md +++ b/.changeset/honest-experts-battle.md @@ -2,4 +2,4 @@ "@hashicorp/design-system-components": patch --- -`AppSideNav` - fixed bug where scrolling was blocked when the `AppSideNav` was expanded on desktop views. +`AppSideNav` - fixed bug where scrolling was blocked when the `AppSideNav` was expanded on desktop views. Also fixed bug where able to focus links that are visually hidden. From 6af08bcf8e30d7a5b9098f5e457d2418d5f462c5 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Tue, 13 May 2025 07:59:24 -0400 Subject: [PATCH 07/17] fix: issue where pressing escape doesnt reset ability to scroll --- packages/components/src/components/hds/app-side-nav/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/components/hds/app-side-nav/index.ts b/packages/components/src/components/hds/app-side-nav/index.ts index 9149bf3f63..fbf4ab13cd 100644 --- a/packages/components/src/components/hds/app-side-nav/index.ts +++ b/packages/components/src/components/hds/app-side-nav/index.ts @@ -161,6 +161,7 @@ export default class HdsAppSideNav extends Component { if (event.key === 'Escape' && !this._isMinimized && !this._isDesktop) { this._isMinimized = true; this.synchronizeInert(); + this.unlockBodyScroll(); } } From 6b4ba9cec52f8b2986ea924aa3531d339cf8fc16 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Tue, 13 May 2025 09:55:11 -0400 Subject: [PATCH 08/17] fix: failing test --- .../components/hds/app-side-nav/index-test.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index 2438544bb9..28353e4432 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -304,11 +304,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { `); - - assert.dom('body', document).hasStyle({ - overflow: 'hidden', - }); - await this.changeBrowserSize(false); assert.deepEqual( calls[1], @@ -316,6 +311,12 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { 'resizing to mobile triggers a false event' ); + await click('.hds-app-side-nav__toggle-button'); + + assert.dom('body', document).hasStyle({ + overflow: 'hidden', + }); + await this.changeBrowserSize(true); assert.deepEqual( calls[2], From 1adbcbd99dcc49d2d724005cd5fec812c99de1b7 Mon Sep 17 00:00:00 2001 From: Lee White Date: Tue, 13 May 2025 14:03:04 -0400 Subject: [PATCH 09/17] Update .changeset/honest-experts-battle.md Co-authored-by: Kristin Bradley --- .changeset/honest-experts-battle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/honest-experts-battle.md b/.changeset/honest-experts-battle.md index c5a78e7fea..cda4de711e 100644 --- a/.changeset/honest-experts-battle.md +++ b/.changeset/honest-experts-battle.md @@ -2,4 +2,4 @@ "@hashicorp/design-system-components": patch --- -`AppSideNav` - fixed bug where scrolling was blocked when the `AppSideNav` was expanded on desktop views. Also fixed bug where able to focus links that are visually hidden. +`AppSideNav` - Fixed bug where scrolling was blocked when the `AppSideNav` was expanded on desktop views. Also fixed bug which allowed user to focus links that were visually hidden. From 800bdb984b2e036da181defed9ee2db4dd709a0b Mon Sep 17 00:00:00 2001 From: shleewhite Date: Tue, 13 May 2025 14:10:00 -0400 Subject: [PATCH 10/17] chore: rename class hds-app-side-nav-hide-when-minimized -> hds-app-side-nav--hide-when-minimized --- .../src/components/hds/app-side-nav/index.hbs | 2 +- .../src/components/hds/app-side-nav/index.ts | 2 +- .../hds/app-side-nav/portal/target.hbs | 2 +- .../styles/components/app-side-nav/main.scss | 2 +- .../components/hds/app-side-nav/index-test.js | 26 +++++++++---------- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/components/src/components/hds/app-side-nav/index.hbs b/packages/components/src/components/hds/app-side-nav/index.hbs index 9d4cc9d2a7..db202c1e3d 100644 --- a/packages/components/src/components/hds/app-side-nav/index.hbs +++ b/packages/components/src/components/hds/app-side-nav/index.hbs @@ -27,7 +27,7 @@ /> {{/if}} -
+
{{~yield~}}
diff --git a/packages/components/src/components/hds/app-side-nav/index.ts b/packages/components/src/components/hds/app-side-nav/index.ts index fbf4ab13cd..3c6c7ed584 100644 --- a/packages/components/src/components/hds/app-side-nav/index.ts +++ b/packages/components/src/components/hds/app-side-nav/index.ts @@ -188,7 +188,7 @@ export default class HdsAppSideNav extends Component { @action didInsert(element: HTMLElement): void { this._containersToHide = element.querySelectorAll( - '.hds-app-side-nav-hide-when-minimized' + '.hds-app-side-nav--hide-when-minimized' ); this._body = document.body; // Store the initial `overflow` value of `` so we can reset to it diff --git a/packages/components/src/components/hds/app-side-nav/portal/target.hbs b/packages/components/src/components/hds/app-side-nav/portal/target.hbs index 8e4d02e2ea..7da3fd1468 100644 --- a/packages/components/src/components/hds/app-side-nav/portal/target.hbs +++ b/packages/components/src/components/hds/app-side-nav/portal/target.hbs @@ -8,7 +8,7 @@ @multiple={{true}} @onChange={{this.panelsChanged}} @name={{if @targetName @targetName "hds-app-side-nav-portal-target"}} - class="hds-app-side-nav__content-panels hds-app-side-nav-hide-when-minimized" + class="hds-app-side-nav__content-panels hds-app-side-nav--hide-when-minimized" {{did-update this.didUpdateSubnav this._numSubnavs}} /> \ No newline at end of file diff --git a/packages/components/src/styles/components/app-side-nav/main.scss b/packages/components/src/styles/components/app-side-nav/main.scss index 9bb53ca59b..f3b95c474c 100644 --- a/packages/components/src/styles/components/app-side-nav/main.scss +++ b/packages/components/src/styles/components/app-side-nav/main.scss @@ -107,7 +107,7 @@ // - hidden (immediately) when the sidenav is "minimized" // - shown (transitioning their opacity) when the sidenav is "expanded" -.hds-app-side-nav-hide-when-minimized { +.hds-app-side-nav--hide-when-minimized { .hds-app-side-nav--is-minimized & { visibility: hidden !important; // we need `!important` here to override the inline style applied to the single panels opacity: 0; diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index 28353e4432..57a41afabb 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -156,7 +156,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { test('it collapses when the ESC key is pressed on narrow viewports', async function (assert) { await render(hbs` - + `); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); await click('.hds-app-side-nav__toggle-button'); @@ -166,7 +166,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await triggerKeyEvent('#test-app-side-nav', 'keydown', 'Escape'); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); - assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); }); // COLLAPSIBLE @@ -191,7 +191,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { test('the "non-minimized" and "minimized" states have impact on its internal properties', async function (assert) { await render(hbs` - + `); assert .dom('#test-app-side-nav') @@ -203,7 +203,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { .dom('.hds-app-side-nav__toggle-button .hds-icon') .hasClass('hds-icon-chevrons-left'); assert - .dom('.hds-app-side-nav-hide-when-minimized') + .dom('.hds-app-side-nav--hide-when-minimized') .doesNotHaveAttribute('inert'); assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); @@ -217,7 +217,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { assert .dom('.hds-app-side-nav__toggle-button .hds-icon') .hasClass('hds-icon-chevrons-right'); - assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); }); @@ -235,7 +235,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - + `); assert.strictEqual(calls.length, 1, 'called with initial viewport'); @@ -247,7 +247,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { 'resizing to mobile triggers a false event' ); - assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); }); test('when collapsed and the viewport changes from mobile to desktop, it automatically expands and is no longer inert', async function (assert) { @@ -263,11 +263,11 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - + `); await click('.hds-app-side-nav__toggle-button'); - assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); await this.changeBrowserSize(false); assert.deepEqual( @@ -275,7 +275,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { [false], 'resizing to mobile triggers a false event' ); - assert.dom('.hds-app-side-nav-hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); await this.changeBrowserSize(true); assert.deepEqual( @@ -284,7 +284,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { 'resizing to desktop triggers a true event' ); assert - .dom('.hds-app-side-nav-hide-when-minimized') + .dom('.hds-app-side-nav--hide-when-minimized') .doesNotHaveAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); }); @@ -302,7 +302,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - + `); await this.changeBrowserSize(false); assert.deepEqual( @@ -334,7 +334,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - + `); await click('.hds-app-side-nav__toggle-button'); From 8dd5c5b0bb59f8602129506a22f25ae4379e4f0d Mon Sep 17 00:00:00 2001 From: shleewhite Date: Tue, 13 May 2025 14:10:16 -0400 Subject: [PATCH 11/17] chore: remove content from docs that isnt true --- .../docs/components/app-side-nav/partials/code/how-to-use.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/website/docs/components/app-side-nav/partials/code/how-to-use.md b/website/docs/components/app-side-nav/partials/code/how-to-use.md index 7ac99d2eba..eedd6468db 100644 --- a/website/docs/components/app-side-nav/partials/code/how-to-use.md +++ b/website/docs/components/app-side-nav/partials/code/how-to-use.md @@ -206,15 +206,13 @@ The App Side Nav component **automatically**: Any other content in the App Side Nav needs to be **explicitly handled** by the consumers (in this way they have full control of the content they add, and they can customize the transition as they want/need). -One possible way to do it is to use the **`hds-app-side-nav-hide-when-minimized` class**. This is a special class that can be applied to a DOM element so that it **automatically** fades in/out when the App Side Nav changes its “minimization” state. +One possible way to do it is to use the **`hds-app-side-nav--hide-when-minimized` class**. This is a special class that can be applied to a DOM element so that it **automatically** fades in/out when the App Side Nav changes its “minimization” state. More specifically: - `minimized → maximized` transition: the content appears with a fade-in effect, when the width animation is already completed (the width is maximized) - `maximized → minimized` transition: the content disappears at once with no transition, before the width animation starts -Another option is to use the **`isMinimized` parameter**, which is useful in those cases where the content is so custom/specialized that it can’t just be faded in/out but needs to have a different kind of transition (eg. remain visible but change layout or respond to the width of the container). This value is passed down by the `<:header/body/footer>` named blocks as parameters, and can be used to build custom logic on the consumers’ side. - #### Advanced customization Some aspects of the responsiveness/animation/transition of the App Side Nav are parameterized in code via CSS custom properties. It means that _in theory_ they could be customized/overwritten. This though is **something that we don’t recommend**. From ffb75637f4c0dd5f0090b35463399befd43780b3 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Wed, 14 May 2025 13:58:18 -0700 Subject: [PATCH 12/17] chore: refactor how content is marked as inert --- .../src/components/hds/app-side-nav/index.hbs | 2 +- .../src/components/hds/app-side-nav/index.ts | 24 +++++++++--------- .../hds/app-side-nav/portal/target.hbs | 2 +- .../styles/components/app-side-nav/main.scss | 7 +----- .../components/hds/app-side-nav/index-test.js | 25 ++++++------------- .../app-side-nav/partials/code/how-to-use.md | 10 +++----- 6 files changed, 26 insertions(+), 44 deletions(-) diff --git a/packages/components/src/components/hds/app-side-nav/index.hbs b/packages/components/src/components/hds/app-side-nav/index.hbs index db202c1e3d..a8a7c140ef 100644 --- a/packages/components/src/components/hds/app-side-nav/index.hbs +++ b/packages/components/src/components/hds/app-side-nav/index.hbs @@ -27,7 +27,7 @@ /> {{/if}} -
+
{{~yield~}}
diff --git a/packages/components/src/components/hds/app-side-nav/index.ts b/packages/components/src/components/hds/app-side-nav/index.ts index 3c6c7ed584..e12273ce20 100644 --- a/packages/components/src/components/hds/app-side-nav/index.ts +++ b/packages/components/src/components/hds/app-side-nav/index.ts @@ -34,7 +34,7 @@ export default class HdsAppSideNav extends Component { private _body!: HTMLElement; private _bodyInitialOverflowValue = ''; private _desktopMQ: MediaQueryList; - private _containersToHide!: NodeListOf; + private _navWrapperBody!: HTMLElement; // we use the `lg` breakpoint for `desktop` viewports, but consumers can override its value private _desktopMQVal = this.args.breakpoint ?? hdsBreakpoints['lg'].px; @@ -123,13 +123,11 @@ export default class HdsAppSideNav extends Component { } synchronizeInert(): void { - this._containersToHide?.forEach((element): void => { - if (this._isMinimized) { - element.setAttribute('inert', ''); - } else { - element.removeAttribute('inert'); - } - }); + if (this._isMinimized) { + this._navWrapperBody?.setAttribute('inert', ''); + } else { + this._navWrapperBody?.removeAttribute('inert'); + } } lockBodyScroll(): void { @@ -186,16 +184,18 @@ export default class HdsAppSideNav extends Component { } @action - didInsert(element: HTMLElement): void { - this._containersToHide = element.querySelectorAll( - '.hds-app-side-nav--hide-when-minimized' - ); + didInsert(): void { this._body = document.body; // Store the initial `overflow` value of `` so we can reset to it this._bodyInitialOverflowValue = this._body.style.getPropertyValue('overflow'); } + @action + didInsertNavWrapperBody(element: HTMLElement): void { + this._navWrapperBody = element; + } + @action setTransition(phase: string, event: TransitionEvent): void { // we only want to respond to `width` animation/transitions diff --git a/packages/components/src/components/hds/app-side-nav/portal/target.hbs b/packages/components/src/components/hds/app-side-nav/portal/target.hbs index 7da3fd1468..1e55cc2686 100644 --- a/packages/components/src/components/hds/app-side-nav/portal/target.hbs +++ b/packages/components/src/components/hds/app-side-nav/portal/target.hbs @@ -8,7 +8,7 @@ @multiple={{true}} @onChange={{this.panelsChanged}} @name={{if @targetName @targetName "hds-app-side-nav-portal-target"}} - class="hds-app-side-nav__content-panels hds-app-side-nav--hide-when-minimized" + class="hds-app-side-nav__content-panels" {{did-update this.didUpdateSubnav this._numSubnavs}} /> \ No newline at end of file diff --git a/packages/components/src/styles/components/app-side-nav/main.scss b/packages/components/src/styles/components/app-side-nav/main.scss index f3b95c474c..1ddb728436 100644 --- a/packages/components/src/styles/components/app-side-nav/main.scss +++ b/packages/components/src/styles/components/app-side-nav/main.scss @@ -102,12 +102,7 @@ overflow-y: auto; } -// "HIDE-WHEN-MINIMIZED" SPECIAL CLASS -// this is a special class that is used to control which elements of the sidenav need to be: -// - hidden (immediately) when the sidenav is "minimized" -// - shown (transitioning their opacity) when the sidenav is "expanded" - -.hds-app-side-nav--hide-when-minimized { +.hds-app-side-nav__wrapper-body { .hds-app-side-nav--is-minimized & { visibility: hidden !important; // we need `!important` here to override the inline style applied to the single panels opacity: 0; diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index 57a41afabb..c6ca37126f 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -156,7 +156,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { test('it collapses when the ESC key is pressed on narrow viewports', async function (assert) { await render(hbs` - `); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); await click('.hds-app-side-nav__toggle-button'); @@ -166,7 +165,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await triggerKeyEvent('#test-app-side-nav', 'keydown', 'Escape'); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); - assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); }); // COLLAPSIBLE @@ -191,7 +190,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { test('the "non-minimized" and "minimized" states have impact on its internal properties', async function (assert) { await render(hbs` - `); assert .dom('#test-app-side-nav') @@ -202,9 +200,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { assert .dom('.hds-app-side-nav__toggle-button .hds-icon') .hasClass('hds-icon-chevrons-left'); - assert - .dom('.hds-app-side-nav--hide-when-minimized') - .doesNotHaveAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').doesNotHaveAttribute('inert'); assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); @@ -217,8 +213,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { assert .dom('.hds-app-side-nav__toggle-button .hds-icon') .hasClass('hds-icon-chevrons-right'); - assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); - assert.dom('#test-app-side-nav-body').doesNotHaveAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); }); @@ -235,7 +230,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - `); assert.strictEqual(calls.length, 1, 'called with initial viewport'); @@ -247,7 +241,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { 'resizing to mobile triggers a false event' ); - assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); }); test('when collapsed and the viewport changes from mobile to desktop, it automatically expands and is no longer inert', async function (assert) { @@ -263,11 +257,10 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - `); await click('.hds-app-side-nav__toggle-button'); - assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); await this.changeBrowserSize(false); assert.deepEqual( @@ -275,7 +268,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { [false], 'resizing to mobile triggers a false event' ); - assert.dom('.hds-app-side-nav--hide-when-minimized').hasAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); await this.changeBrowserSize(true); assert.deepEqual( @@ -283,9 +276,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { [true], 'resizing to desktop triggers a true event' ); - assert - .dom('.hds-app-side-nav--hide-when-minimized') - .doesNotHaveAttribute('inert'); + assert.dom('.hds-app-side-nav__wrapper-body').doesNotHaveAttribute('inert'); assert.dom('body', document).doesNotHaveStyle('overflow'); }); @@ -302,7 +293,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - `); await this.changeBrowserSize(false); assert.deepEqual( @@ -334,7 +324,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > - `); await click('.hds-app-side-nav__toggle-button'); diff --git a/website/docs/components/app-side-nav/partials/code/how-to-use.md b/website/docs/components/app-side-nav/partials/code/how-to-use.md index eedd6468db..0bb9eb77e3 100644 --- a/website/docs/components/app-side-nav/partials/code/how-to-use.md +++ b/website/docs/components/app-side-nav/partials/code/how-to-use.md @@ -201,18 +201,16 @@ Each one of these states has CSS class names associated, and they’re used by t The App Side Nav component **automatically**: -- fades in/out the “actions” block in the header and the content injected in the body via “portals”. +- fades in/out the the content in the navigation - swaps the toggle button icon from “menu” to “close” and moves it from one position to another -Any other content in the App Side Nav needs to be **explicitly handled** by the consumers (in this way they have full control of the content they add, and they can customize the transition as they want/need). - -One possible way to do it is to use the **`hds-app-side-nav--hide-when-minimized` class**. This is a special class that can be applied to a DOM element so that it **automatically** fades in/out when the App Side Nav changes its “minimization” state. - -More specifically: +More specifically, the animation is: - `minimized → maximized` transition: the content appears with a fade-in effect, when the width animation is already completed (the width is maximized) - `maximized → minimized` transition: the content disappears at once with no transition, before the width animation starts +Any other content in the App Side Nav needs to be **explicitly handled** by the consumers (in this way they have full control of the content they add, and they can customize the transition as they want/need). + #### Advanced customization Some aspects of the responsiveness/animation/transition of the App Side Nav are parameterized in code via CSS custom properties. It means that _in theory_ they could be customized/overwritten. This though is **something that we don’t recommend**. From d0f4983c434ff969c89d1dd8cd7cbbb64e9b0e0c Mon Sep 17 00:00:00 2001 From: shleewhite Date: Wed, 14 May 2025 14:15:16 -0700 Subject: [PATCH 13/17] chore: typo --- .../docs/components/app-side-nav/partials/code/how-to-use.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/components/app-side-nav/partials/code/how-to-use.md b/website/docs/components/app-side-nav/partials/code/how-to-use.md index 0bb9eb77e3..b84944ee22 100644 --- a/website/docs/components/app-side-nav/partials/code/how-to-use.md +++ b/website/docs/components/app-side-nav/partials/code/how-to-use.md @@ -201,7 +201,7 @@ Each one of these states has CSS class names associated, and they’re used by t The App Side Nav component **automatically**: -- fades in/out the the content in the navigation +- fades in/out the content in the navigation - swaps the toggle button icon from “menu” to “close” and moves it from one position to another More specifically, the animation is: From 45d098d37f741c3f57660170adac23afc990f225 Mon Sep 17 00:00:00 2001 From: Lee White Date: Wed, 14 May 2025 16:29:24 -0700 Subject: [PATCH 14/17] Update showcase/tests/integration/components/hds/app-side-nav/index-test.js Co-authored-by: Cristiano Rastelli --- .../tests/integration/components/hds/app-side-nav/index-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index c6ca37126f..4c69b970bd 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -324,7 +324,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @onDesktopViewportChange={{this.onDesktopViewportChange}} > -`); +`); await click('.hds-app-side-nav__toggle-button'); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); From f479157c38e6c70fb7b711b24a7cdfdd1aa4a1c8 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Wed, 14 May 2025 17:18:23 -0700 Subject: [PATCH 15/17] chore: remove unneeded content inside AppSideNav tests --- .../components/hds/app-side-nav/index-test.js | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index 4c69b970bd..14559b1a91 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -154,9 +154,8 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { }); test('it collapses when the ESC key is pressed on narrow viewports', async function (assert) { - await render(hbs` - -`); + await render(hbs``); assert.dom('#test-app-side-nav').hasClass('hds-app-side-nav--is-minimized'); await click('.hds-app-side-nav__toggle-button'); assert @@ -228,9 +227,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await render(hbs` - -`); +/>`); assert.strictEqual(calls.length, 1, 'called with initial viewport'); @@ -255,9 +252,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await render(hbs` - -`); +/>`); await click('.hds-app-side-nav__toggle-button'); assert.dom('.hds-app-side-nav__wrapper-body').hasAttribute('inert'); @@ -291,9 +286,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await render(hbs` - -`); +/>`); await this.changeBrowserSize(false); assert.deepEqual( calls[1], @@ -323,7 +316,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { @isCollapsible={{true}} @onDesktopViewportChange={{this.onDesktopViewportChange}} > - + `); await click('.hds-app-side-nav__toggle-button'); @@ -331,7 +324,7 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { assert.dom('.hds-app-side-nav__toggle-button').isFocused(); await tab(); - assert.dom('#button-2').isFocused(); + assert.dom('#button-outside').isFocused(); }); // CALLBACKS From 0e325c622a82765567649a4e75bee2bdf0307703 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Wed, 14 May 2025 17:21:51 -0700 Subject: [PATCH 16/17] chore: remove unneeded arg in test --- .../tests/integration/components/hds/app-side-nav/index-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/showcase/tests/integration/components/hds/app-side-nav/index-test.js b/showcase/tests/integration/components/hds/app-side-nav/index-test.js index 14559b1a91..dac4312156 100644 --- a/showcase/tests/integration/components/hds/app-side-nav/index-test.js +++ b/showcase/tests/integration/components/hds/app-side-nav/index-test.js @@ -314,7 +314,6 @@ module('Integration | Component | hds/app-side-nav/index', function (hooks) { await render(hbs` `); From 345c5509093eec4fe7b7071936e92d3de3a155f2 Mon Sep 17 00:00:00 2001 From: shleewhite Date: Wed, 14 May 2025 17:32:43 -0700 Subject: [PATCH 17/17] fix: use custom modifiers instead of did-insert --- .../src/components/hds/app-side-nav/index.hbs | 4 +-- .../src/components/hds/app-side-nav/index.ts | 25 +++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/components/src/components/hds/app-side-nav/index.hbs b/packages/components/src/components/hds/app-side-nav/index.hbs index a8a7c140ef..aa8341a956 100644 --- a/packages/components/src/components/hds/app-side-nav/index.hbs +++ b/packages/components/src/components/hds/app-side-nav/index.hbs @@ -10,7 +10,7 @@ {{on "transitionend" (fn this.setTransition "end")}} {{! @glint-expect-error - https://github.com/josemarluedke/ember-focus-trap/issues/86 }} {{focus-trap isActive=this.shouldTrapFocus}} - {{did-insert this.didInsert}} + {{this._setUpBodyElement}} >

Application local navigation

@@ -27,7 +27,7 @@ /> {{/if}} -
+
{{~yield~}}
diff --git a/packages/components/src/components/hds/app-side-nav/index.ts b/packages/components/src/components/hds/app-side-nav/index.ts index e12273ce20..6266474dcf 100644 --- a/packages/components/src/components/hds/app-side-nav/index.ts +++ b/packages/components/src/components/hds/app-side-nav/index.ts @@ -8,6 +8,7 @@ import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; import { registerDestructor } from '@ember/destroyable'; import type Owner from '@ember/owner'; +import { modifier } from 'ember-modifier'; import { hdsBreakpoints } from '@hashicorp/design-system-components/utils/hds-breakpoints'; @@ -49,6 +50,17 @@ export default class HdsAppSideNav extends Component { }); } + private _setUpBodyElement = modifier(() => { + this._body = document.body; + // Store the initial `overflow` value of `` so we can reset to it + this._bodyInitialOverflowValue = + this._body.style.getPropertyValue('overflow'); + }); + + private _setUpNavWrapperBody = modifier((element: HTMLElement) => { + this._navWrapperBody = element; + }); + addEventListeners(): void { // eslint-disable-next-line @typescript-eslint/unbound-method document.addEventListener('keydown', this.escapePress, true); @@ -183,19 +195,6 @@ export default class HdsAppSideNav extends Component { } } - @action - didInsert(): void { - this._body = document.body; - // Store the initial `overflow` value of `` so we can reset to it - this._bodyInitialOverflowValue = - this._body.style.getPropertyValue('overflow'); - } - - @action - didInsertNavWrapperBody(element: HTMLElement): void { - this._navWrapperBody = element; - } - @action setTransition(phase: string, event: TransitionEvent): void { // we only want to respond to `width` animation/transitions