From 01c2468f86aca28dadbe751bba2e8956ad70d6ba Mon Sep 17 00:00:00 2001 From: Priya Patel Date: Thu, 17 Apr 2025 10:57:45 -0700 Subject: [PATCH] Refactor: Replace Rose::Dialog with HDS::Modal for desktop client (#2766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 rose dialog to hds modal * fix: 🐛 dark theme hds overlay * fix: 🐛 dark mode hds modal shadow * fix: 🐛 move dark mode modal override to a mixin * fix: 🐛 update exists to isVisible --- .../styles/hds/themes/dark-mode/index.scss | 16 ++++ ui/admin/tests/acceptance/policy/list-test.js | 4 +- .../tests/acceptance/sessions/list-test.js | 4 +- .../tests/acceptance/targets/list-test.js | 4 +- ui/admin/tests/helpers/selectors.js | 3 +- ui/desktop/app/templates/application.hbs | 95 ++++++++++--------- .../acceptance/projects/targets/index-test.js | 21 ++-- .../projects/targets/target-test.js | 38 ++++---- 8 files changed, 105 insertions(+), 80 deletions(-) diff --git a/addons/rose/addon/styles/hds/themes/dark-mode/index.scss b/addons/rose/addon/styles/hds/themes/dark-mode/index.scss index a69c914a0..a210e5ae3 100644 --- a/addons/rose/addon/styles/hds/themes/dark-mode/index.scss +++ b/addons/rose/addon/styles/hds/themes/dark-mode/index.scss @@ -107,12 +107,28 @@ ); } +@mixin dark-mode-modal-override { + // override for HDS dialog overlay to keep darker overlay + /* stylelint-disable-next-line selector-class-pattern */ + .hds-dialog-primitive__overlay { + background-color: var(--token-color-palette-neutral-0); + } + + // override for HDS modal to remove high contrast shadow and set border + .hds-modal { + border: 1px solid var(--token-color-border-strong); + box-shadow: none; + } +} + @media (prefers-color-scheme: dark) { .ember-application:not(.rose-theme-light) { @include dark-mode; + @include dark-mode-modal-override; } } .ember-application.rose-theme-dark { @include dark-mode; + @include dark-mode-modal-override; } diff --git a/ui/admin/tests/acceptance/policy/list-test.js b/ui/admin/tests/acceptance/policy/list-test.js index 503bd8740..c11399a53 100644 --- a/ui/admin/tests/acceptance/policy/list-test.js +++ b/ui/admin/tests/acceptance/policy/list-test.js @@ -89,9 +89,7 @@ module('Acceptance | policies | list', function (hooks) { 'policies' ].includes('create'), ); - assert - .dom(commonSelectors.SIDEBAR_NAV_LINK(urls.policies)) - .doesNotExist(); + assert.dom(commonSelectors.SIDEBAR_NAV_LINK(urls.policies)).doesNotExist(); await visit(urls.policies); diff --git a/ui/admin/tests/acceptance/sessions/list-test.js b/ui/admin/tests/acceptance/sessions/list-test.js index 4571f8149..769a1bfa9 100644 --- a/ui/admin/tests/acceptance/sessions/list-test.js +++ b/ui/admin/tests/acceptance/sessions/list-test.js @@ -126,9 +126,7 @@ module('Acceptance | sessions | list', function (hooks) { ), ); - assert - .dom(commonSelectors.SIDEBAR_NAV_LINK(urls.sessions)) - .doesNotExist(); + assert.dom(commonSelectors.SIDEBAR_NAV_LINK(urls.sessions)).doesNotExist(); }); test('users can navigate to sessions with proper authorization', async function (assert) { diff --git a/ui/admin/tests/acceptance/targets/list-test.js b/ui/admin/tests/acceptance/targets/list-test.js index 029437179..ac9b2cec0 100644 --- a/ui/admin/tests/acceptance/targets/list-test.js +++ b/ui/admin/tests/acceptance/targets/list-test.js @@ -123,7 +123,9 @@ module('Acceptance | targets | list', function (hooks) { 'create', ), ); - assert.dom(commonSelectors.SIDEBAR_NAV_CONTENT).doesNotIncludeText('Targets'); + assert + .dom(commonSelectors.SIDEBAR_NAV_CONTENT) + .doesNotIncludeText('Targets'); }); test('user can navigate to index with only create action', async function (assert) { diff --git a/ui/admin/tests/helpers/selectors.js b/ui/admin/tests/helpers/selectors.js index a9ed2a376..83c3e3295 100644 --- a/ui/admin/tests/helpers/selectors.js +++ b/ui/admin/tests/helpers/selectors.js @@ -35,7 +35,8 @@ export const PAGE_MESSAGE_LINK = export const HREF = (url) => `[href="${url}"]`; -export const SIDEBAR_NAV_LINK = (url) => `.hds-side-nav__content a[href="${url}"]`; +export const SIDEBAR_NAV_LINK = (url) => + `.hds-side-nav__content a[href="${url}"]`; export const SIDEBAR_NAV_CONTENT = '.hds-side-nav__content'; export const SIDEBAR_USER_DROPDOWN = '[data-test-side-nav-user-dropdown] button'; diff --git a/ui/desktop/app/templates/application.hbs b/ui/desktop/app/templates/application.hbs index c1e9d3c34..e59862787 100644 --- a/ui/desktop/app/templates/application.hbs +++ b/ui/desktop/app/templates/application.hbs @@ -86,53 +86,58 @@ {{#if confirmation.options.isConnectError}} - - -

+ + + {{t 'states.error'}} + + + {{confirmation.text}} -

-

+ +
+ {{t 'questions.retry-confirm'}} -

- - - - {{t 'actions.retry'}} - - - {{t 'actions.cancel'}} - - - + + + + + + + + + {{else}} - - -

{{confirmation.text}}

- - - - {{t 'actions.ok'}} - - - {{t 'actions.cancel'}} - - - + + + {{t 'actions.confirm'}} + + + + {{confirmation.text}} + + + + + + + + + {{/if}} \ No newline at end of file diff --git a/ui/desktop/tests/acceptance/projects/targets/index-test.js b/ui/desktop/tests/acceptance/projects/targets/index-test.js index 83cde8662..ac9f006ba 100644 --- a/ui/desktop/tests/acceptance/projects/targets/index-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/index-test.js @@ -31,12 +31,13 @@ module('Acceptance | projects | targets | index', function (hooks) { const APP_STATE_TITLE = '.hds-application-state__title'; const TARGET_DETAILS_ROUTE_NAME = 'scopes.scope.projects.targets.target.index'; - const ROSE_DIALOG_MODAL = '.rose-dialog-error'; + const HDS_DIALOG_MODAL = '.hds-modal'; const CHOOSE_HOST_MODAL = '[data-test-host-modal]'; - const ROSE_DIALOG_MODAL_BUTTONS = '.rose-dialog-footer button'; - const ROSE_DIALOG_RETRY_BUTTON = '.rose-dialog footer .rose-button-primary'; - const ROSE_DIALOG_CANCEL_BUTTON = - '.rose-dialog footer .rose-button-secondary'; + const HDS_DIALOG_MODAL_BUTTONS = '.hds-modal__footer button'; + const HDS_DIALOG_RETRY_BUTTON = + '.hds-modal__footer .hds-button--color-primary'; + const HDS_DIALOG_CANCEL_BUTTON = + '.hds-modal__footer .hds-button--color-secondary'; const SESSIONS_FLYOUT = '[data-test-targets-sessions-flyout]'; const SESSIONS_FLYOUT_TITLE = '[data-test-targets-sessions-flyout] .hds-flyout__title'; @@ -273,7 +274,7 @@ module('Acceptance | projects | targets | index', function (hooks) { await click(`[data-test-targets-connect-button="${instances.target.id}"]`); - assert.dom(ROSE_DIALOG_MODAL).exists(); + assert.dom(HDS_DIALOG_MODAL).isVisible(); assert.dom(CHOOSE_HOST_MODAL).doesNotExist(); assert.strictEqual(currentRouteName(), TARGET_DETAILS_ROUTE_NAME); }); @@ -317,10 +318,10 @@ module('Acceptance | projects | targets | index', function (hooks) { await click(`[data-test-targets-connect-button="${instances.target.id}"]`); assert.strictEqual(currentURL(), urls.targets); - assert.dom(ROSE_DIALOG_MODAL).exists(); - assert.dom(ROSE_DIALOG_MODAL_BUTTONS).exists({ count: 2 }); - assert.dom(ROSE_DIALOG_RETRY_BUTTON).hasText('Retry'); - assert.dom(ROSE_DIALOG_CANCEL_BUTTON).hasText('Cancel'); + assert.dom(HDS_DIALOG_MODAL).isVisible(); + assert.dom(HDS_DIALOG_MODAL_BUTTONS).isVisible({ count: 2 }); + assert.dom(HDS_DIALOG_RETRY_BUTTON).hasText('Retry'); + assert.dom(HDS_DIALOG_CANCEL_BUTTON).hasText('Cancel'); }); test('user can open sessions flyout when target has active or pending sessions', async function (assert) { diff --git a/ui/desktop/tests/acceptance/projects/targets/target-test.js b/ui/desktop/tests/acceptance/projects/targets/target-test.js index 91405be65..6a03a78b5 100644 --- a/ui/desktop/tests/acceptance/projects/targets/target-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/target-test.js @@ -20,11 +20,12 @@ module('Acceptance | projects | targets | target', function (hooks) { const TARGET_QUICK_CONNECT_BUTTON = '[data-test-host-quick-connect]'; const TARGET_HOST_CONNECT_BUTTON = '[data-test-host-connect]'; const APP_STATE_TITLE = '.hds-application-state__title'; - const ROSE_DIALOG_MODAL = '.rose-dialog-error'; - const ROSE_DIALOG_MODAL_BUTTONS = '.rose-dialog-footer button'; - const ROSE_DIALOG_RETRY_BUTTON = '.rose-dialog footer .rose-button-primary'; - const ROSE_DIALOG_CANCEL_BUTTON = - '.rose-dialog footer .rose-button-secondary'; + const HDS_DIALOG_MODAL = '.hds-modal'; + const HDS_DIALOG_MODAL_BUTTONS = '.hds-modal__footer button'; + const HDS_DIALOG_RETRY_BUTTON = + '.hds-modal__footer .hds-button--color-primary'; + const HDS_DIALOG_CANCEL_BUTTON = + '.hds-modal__footer .hds-button--color-secondary'; const instances = { scopes: { @@ -164,10 +165,10 @@ module('Acceptance | projects | targets | target', function (hooks) { await click(`[href="${urls.target}"]`); await click(TARGET_CONNECT_BUTTON); - assert.dom(ROSE_DIALOG_MODAL).exists(); - assert.dom(ROSE_DIALOG_MODAL_BUTTONS).exists({ count: 2 }); - assert.dom(ROSE_DIALOG_RETRY_BUTTON).hasText('Retry'); - assert.dom(ROSE_DIALOG_CANCEL_BUTTON).hasText('Cancel'); + assert.dom(HDS_DIALOG_MODAL).isVisible(); + assert.dom(HDS_DIALOG_MODAL_BUTTONS).isVisible({ count: 2 }); + assert.dom(HDS_DIALOG_RETRY_BUTTON).hasText('Retry'); + assert.dom(HDS_DIALOG_CANCEL_BUTTON).hasText('Cancel'); }); test('handles connect error', async function (assert) { @@ -181,10 +182,10 @@ module('Acceptance | projects | targets | target', function (hooks) { await click(`[href="${urls.target}"]`); await click(TARGET_CONNECT_BUTTON); - assert.dom(ROSE_DIALOG_MODAL).exists(); - assert.dom(ROSE_DIALOG_MODAL_BUTTONS).exists({ count: 2 }); - assert.dom(ROSE_DIALOG_RETRY_BUTTON).hasText('Retry'); - assert.dom(ROSE_DIALOG_CANCEL_BUTTON).hasText('Cancel'); + assert.dom(HDS_DIALOG_MODAL).isVisible(); + assert.dom(HDS_DIALOG_MODAL_BUTTONS).isVisible({ count: 2 }); + assert.dom(HDS_DIALOG_RETRY_BUTTON).hasText('Retry'); + assert.dom(HDS_DIALOG_CANCEL_BUTTON).hasText('Cancel'); }); test('user can retry on error', async function (assert) { @@ -197,11 +198,14 @@ module('Acceptance | projects | targets | target', function (hooks) { await visit(urls.target); await click(TARGET_CONNECT_BUTTON); - const firstErrorDialog = find(ROSE_DIALOG_MODAL); - await click(ROSE_DIALOG_RETRY_BUTTON, 'Retry'); - const secondErrorDialog = find(ROSE_DIALOG_MODAL); + const firstErrorDialog = find(HDS_DIALOG_MODAL); + await click(HDS_DIALOG_RETRY_BUTTON, 'Retry'); + const secondErrorDialog = find(HDS_DIALOG_MODAL); - assert.notEqual(secondErrorDialog.id, firstErrorDialog.id); + assert.notEqual( + secondErrorDialog.getAttribute('aria-labelledby'), + firstErrorDialog.getAttribute('aria-labelledby'), + ); }); test('user can connect to a target with one host', async function (assert) {