feat: Add dialog component by amanabiy · Pull Request #4904 · cloudscape-design/components

amanabiy · 2026-08-13T13:50:20Z

Description

Adds an inline, non-modal Dialog component for in-context prompts, authorizations, and feedback. Dialog supports header actions, content, and footer slots, keeps the surrounding page interactive, manages initial and restored focus, and supports dismissal through the close button and Escape.

Related links, issue #, if available: 6f6tUHjL27wB - doc

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

Changes include appropriate documentation updates.
Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
Changes do not include unsupported browser features, see CONTRIBUTING.md.
Changes were manually tested for accessibility, see accessibility guidelines.

Security

If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

Changes are covered with new/existing unit tests?
Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2026-08-13T14:02:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.67%. Comparing base (1562d7b) to head (d4727e1).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4904   +/-   ##
=======================================
  Coverage   97.66%   97.67%           
=======================================
  Files         959      963    +4     
  Lines       31345    31421   +76     
  Branches    11579    11605   +26     
=======================================
+ Hits        30614    30690   +76     
+ Misses        724      685   -39     
- Partials        7       46   +39

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

🟡 Changes recommended

The updated Style API docs snapshot indicates an empty generated docs set (likely breaking snapshot tests) and the Dialog dismiss button can render without an accessible label when dismissAriaLabel is omitted.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new Dialog component to the Cloudscape React component library, including styling backed by design tokens, testing utilities, and example pages used for visual/integ validation.

Changes:

Introduces Dialog (public component + internal implementation), styles, test classes, and unit tests.
Extends design tokens (spacing/borders/colors/shadows + metadata + token name unions) to support the Dialog surface and layout.
Adds test-utils DOM wrapper support for Dialog and updates snapshots/examples pages accordingly.

File summaries

File	Description
style-dictionary/visual-refresh/spacing.ts	Adds Dialog spacing tokens mapped to existing spacing primitives.
style-dictionary/visual-refresh/shadows.ts	Adds `shadowDialog` token.
style-dictionary/visual-refresh/metadata/spacing.ts	Adds metadata entries for new Dialog spacing tokens.
style-dictionary/visual-refresh/metadata/shadows.ts	Adds metadata for `shadowDialog`.
style-dictionary/visual-refresh/metadata/colors.ts	Updates Dialog color token descriptions; adds `colorBorderDialogTop` metadata.
style-dictionary/visual-refresh/metadata/borders.ts	Adds metadata for Dialog border radius/width and top border width.
style-dictionary/visual-refresh/colors.ts	Updates Dialog background/border mappings; adds `colorBorderDialogTop`.
style-dictionary/visual-refresh/borders.ts	Adds Dialog border radius/width tokens and top border width token.
style-dictionary/utils/token-names.ts	Extends token name unions for new Dialog tokens.
src/test-utils/dom/dialog/index.ts	Adds `DialogWrapper` for DOM test-utils.
src/dialog/test-classes/styles.scss	Adds test class hooks for Dialog sub-elements.
src/dialog/styles.scss	Implements Dialog styling using design tokens (padding, borders, separators).
src/dialog/internal.tsx	Implements Dialog behavior (focus management, Escape handling, dismiss button).
src/dialog/interfaces.ts	Defines public Dialog props and event detail types.
src/dialog/index.tsx	Exports public `Dialog` component wiring base-component metadata.
src/dialog/tests/dialog.test.tsx	Adds unit tests for rendering, accessibility labeling, dismiss behavior, and focus restoration.
src/tests/snapshot-tests/snapshots/test-utils-wrappers.test.tsx.snap	Updates test-utils wrappers snapshot to include Dialog wrapper APIs.
src/tests/snapshot-tests/snapshots/test-utils-selectors.test.tsx.snap	Updates test-utils selectors snapshot to include Dialog selectors.
src/tests/snapshot-tests/snapshots/style-docs.test.ts.snap	Updates Style API docs snapshot output (currently shows empty list).
src/tests/snapshot-tests/snapshots/documenter.test.ts.snap	Adds generated component docs snapshot for Dialog + wrapper docs snapshot.
src/tests/snapshot-tests/snapshots/design-tokens.test.ts.snap	Updates design token artifact snapshots reflecting new/changed Dialog tokens.
src/integ/snapshots/themes.test.ts.snap	Updates theme custom-properties snapshots to include new Dialog tokens/values.
pages/dialog/simple.page.tsx	Adds a basic in-flow Dialog demo page.
pages/dialog/permutations.page.tsx	Adds a permutations page to exercise layout combinations.
pages/dialog/inline-feedback.page.tsx	Adds an inline-feedback use-case demo for Dialog.
pages/dialog/follow-up-questions.page.tsx	Adds a follow-up questions (pagination in header actions) demo.
pages/dialog/common.tsx	Adds shared layout wrapper for Dialog demo pages.
pages/dialog/authorization.page.tsx	Adds an authorization flow demo for Dialog.
build-tools/utils/pluralize.js	Adds pluralization mapping for “Dialog” to “Dialogs”.

Review details

Suppressed comments (1)

src/tests/snapshot-tests/snapshots/style-docs.test.ts.snap:4

This snapshot update indicates that the generated Style API docs directory contained no component docs ([]). That contradicts the test behavior, which throws when no Style API docs are present, and would also drop existing snapshots (alert/badge/button). This looks like an accidental snapshot regeneration without running the Style API docs build step; please regenerate after gulp styleDocs (and ensure the component list is non-empty) or revert this snapshot change.

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Style API docs list of components with a Style API doc matches the snapshot 1`] = `[]`;

Files reviewed: 29/29 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

There are blocking issues affecting API typing, i18n completeness for non-English locales (accessibility impact), and an inconsistent Style API docs snapshot update that likely breaks snapshot testing.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 33/33 changed files
Comments generated: 3
Review effort level: Lite

+  "dialog": {
+    "i18nStrings.dismissAriaLabel": "Close dialog"
+  },


chore: clean up fix: add dismissAriaLabel

pan-kot · 2026-09-09T04:51:55Z

+  </SpaceBetween>
+);
+
+// The close button is always present. The closed playground state renders no Dialog,


nit: Let's remove this comment - this looks to be an agentic ai response to a prompt.

pan-kot · 2026-09-09T04:54:32Z

+          {open && (
+            <Dialog
+              header="What's your main goal?"
+              i18nStrings={{ dismissAriaLabel: 'Close' }}


nit: passing i18n={} to the SimplePage helper injects IntlProvider - which should populate all i18nStrings.

pan-kot · 2026-09-09T04:56:56Z

+
+// Minimal conditional-rendering example. Opening Dialog moves focus from the
+// trigger into Dialog. Closing Dialog restores focus to the trigger.
+export default function DialogSimplePage() {


I'd recommend to include a setting or a separate page to imitate a case when the trigger is no longer available after dialog opens (or it opens programmatically, so there is no trigger in the first place). The page should show how to handle focus manually. I'd be nice to secure both cases with an integration test.

pan-kot · 2026-09-09T06:36:18Z

+}
+
+describe('Dialog', () => {
+  describe('root element', () => {


nit: I'd recommend to make the structure of tests more flat: are those little groups like "root element" or "header" really needed?

pan-kot · 2026-09-09T06:37:37Z

+      expect(wrapper.getElement()).toHaveTextContent('content');
+    });
+
+    test('does not set aria-modal', () => {


Why do we need this test? It seems random: if we were to capture everything that the dialog components does not set - it'd be a long list.

pan-kot · 2026-09-09T06:39:03Z

+      );
+    });
+
+    test('does not render content when children are not provided', () => {


nit: I'd question the practicality of this test, as it is hard to imagine a real use case when the dialog without content is needed.

pan-kot · 2026-09-09T06:44:52Z

+
+export namespace DialogProps {
+  export interface DismissDetail {
+    reason: string;


Do we call this field "reason" in some other components? E.g. in the Drawer a similar prop is called "method".

Why does it have a string type and not a union of possible values?

Also take a look at how we name the values in other components - e.g. should we use "closeButton" or "close-button" (I'd argue we also need to call it "dismiss-button" to match the terminology used in the component).

Thinking some more about dialog dismissal - is there any good reason to prevent dialog from closing on-escape, while keeping the dismiss button? If we don't have any use cases - let's remove the property, too.

If in the future we allow hiding the dismiss button - then we should disable the on-escape dismissal, too - relying on consumers to provide it if needed.

pan-kot · 2026-09-09T06:48:23Z

+      expect(onDismiss).toHaveBeenCalledWith(expect.objectContaining({ detail: { reason: 'closeButton' } }));
+    });
+
+    test('fires onDismiss with the keyboard reason and restores focus when Escape is pressed', async () => {


Why do we use reason="keyboard" and not reason="escape"? This can be misleading, as one might think it means that the user activated the dismiss button by focusing it with the keyboard and then pressing Space or Enter.

pan-kot · 2026-09-09T06:51:06Z

+      expect(onDismiss).toHaveBeenCalledWith(expect.objectContaining({ detail: { reason: 'closeButton' } }));
+    });
+
+    test('fires onDismiss with the keyboard reason and restores focus when Escape is pressed', async () => {


Why do we combine focus management and event details check in one test? The focus management should not depend on the dismiss method - we should ensure it happens in both cases: when the user presses Escape or clicks on the dismiss button. We should either modify the prev test to also include focus check, or create separate tests to capture all four behaviours.

pan-kot · 2026-09-09T06:53:28Z

+      });
+
+      expect(document.activeElement).toBe(activeElement);
+      expect(wrapper.findDismissButton()!.getElement()).not.toHaveFocus();


why do we need this second assertion?

What is the purpose of this test, was it created for test coverage?

When the dialog is rendered - it is expected to obtain focus. When it is not rendered - then there is no concern. In this test we render the dialog while breaking one of its internal dependencies - this is not a good proxy for any real use case.

pan-kot · 2026-09-09T06:59:20Z

+      expect(document.activeElement).toBe(wrapper.findDismissButton()!.getElement());
+    });
+
+    test('moves focus to the first headerActions control when present', () => {


Why are we doing that?

Can we instead consistently move focus e.g. to the dialog body - so that it announces its role and the header, instead of announcing either the dismiss or first action label, depending on its configuration?

We use this approach for standalone drawers: https://cloudscape.design/components/drawer/?tabId=playground&example=fixed-position-with-backdrop-and-close-action

pan-kot · 2026-09-09T07:01:59Z

+      const { container, unmount } = render(
+        <Dialog
+          header="Title"
+          headerActions={<button autoFocus={true}>Action</button>}


From the previous test it is clear that the first header action is getting focused by the dialog in either case - so I'd recommend experimenting with a second one.

I would also be nice to see how this behaviour works in different browsers, as there can be some race conditions at play.

pan-kot · 2026-09-09T07:04:37Z

+
+      unmount();
+      await new Promise(resolve => setTimeout(resolve, 0));
+      expect(trigger).not.toHaveFocus();


What is the motivation for not restoring focus? As the dialog closes - the element to restore focus to will no longer be available, but that is also true for cases when the dialog is opened programmatically, or when the trigger is unmounted after click. Should we make the behaviour more predicable by always restoring focus if the prev focused element is still in DOM? When it is not - it is then consumer responsibility to forward it anyways.

pan-kot · 2026-09-09T07:06:22Z

+      trigger.remove();
+    });
+
+    test('moves focus in on mount and restores focus on unmount', async () => {


There are already tests that assert initial focus transition and recovery - what is unique about this one?

pan-kot · 2026-09-09T07:07:38Z

+      trigger.focus();
+      expect(document.activeElement).toBe(trigger);
+
+      rerender(view(true));


Should we instead do the following check:

Open the dialog

Ensure the focus is on the dismiss button

Move focus to another element

Re-render the dialog and ensure the focus stays on another element (the dialog should not steal focus at that point)

pan-kot · 2026-09-09T07:12:09Z

+    });
+
+    test('restores the original trigger when the consumer moves focus while dismissing', async () => {
+      function Harness() {


nit: can we create some renderStatefulDialog function to include the trigger and additional focus target - so that we don't have to create custom components in tests?

Like this:

function renderStatefulDialog({ open, ...dialogProps }) { const [isOpen, setIsOpen] = useState(open); const { container } = render( <> <button data-testid="trigger" /> <button data-testid="focus-target" /> {isOpen ? <Dialog {...dialogProps} /> : null} <> ); return { container, dialog: createWrapper().findDialog() } }

(note that we don't have to use focusTargetRef as the required element can be found by testid on the fly).

pan-kot · 2026-09-09T07:14:19Z

+    props: {},
+    metadata: {
+      hasHeaderActions: Boolean(props.headerActions),
+      hasContent: Boolean(props.children),


Why do we need hasContent - do we anticipate use cases where content is not present?

pan-kot · 2026-09-09T07:16:03Z

+
+  /**
+   * Actions displayed in the header row, before the always-present close button.
+   * Use this for controls related to the dialog, such as a compact `Pagination`.


nit: Do we really need to mention the compact pagination here? It is one of our use cases, which will be surfaced in the component guidelines anyways. This line in the api docs feels random.

pan-kot · 2026-09-09T07:16:50Z

+  headerActions?: React.ReactNode;
+
+  /**
+   * Content of the dialog. The dialog is a shell: the interactive content it hosts


"The dialog is a shell" - what does it mean? Do we use this terminology in any other similar components, e.g. modal or container?

pan-kot · 2026-09-09T07:21:28Z

+
+  export interface I18nStrings {
+    /**
+     * Specifies the ARIA label for the dismiss button.


This description repeats the one we gave for i18nStrings, and it is not parsed by the Documenter. Let's remove it.

pan-kot · 2026-09-09T07:37:35Z

+  ...restProps
+}: InternalDialogProps) {
+  const baseProps = getBaseProps(restProps);
+  const isRefresh = useVisualRefresh();


The visual refresh is our default theme - we should not use this util for new components or features.

pan-kot · 2026-09-09T07:40:24Z

+    const activeElement = dialog.ownerDocument.activeElement;
+    const HTMLElementConstructor = dialog.ownerDocument.defaultView?.HTMLElement;
+    const restoreTarget =
+      HTMLElementConstructor && activeElement instanceof HTMLElementConstructor && !dialog.contains(activeElement)


What is HTMLElementConstructor and why are we doing this?

Can we use the same logic to restore focus as we use in the modal? If we cannot use the focus-lock component - we can at least use the same code, by either copying or extracting the relevant parts.

pan-kot · 2026-09-09T07:45:43Z

+        : null;
+
+    // Focus the first headerActions control when present, otherwise the
+    // always-present close button. The heading is not a focus target.


This description is technically incorrect: the dialog's header type is ReactNode - which means it can include interactive elements, too. I don't think this line requires a description - but most importantly I don't think that this way of handling focus is good (see my suggestion above on focusing the dialog body instead).

pan-kot · 2026-09-09T07:48:11Z

+      if (restoreTarget?.isConnected) {
+        // Cleanup can run synchronously while onDismiss is still executing. Defer
+        // restoration so focus reliably returns to the original trigger after the handler finishes.
+        dialog.ownerDocument.defaultView?.setTimeout(() => {


why do we call setTimeout from dialog.ownerDocument.defaultView?

pan-kot · 2026-09-09T07:56:05Z

+    public: false,
+    themeable: true,
+  },
+  borderDialogTopWidth: {


There is no outside visual refresh. We should remove this var.

pan-kot · 2026-09-09T07:57:07Z

  colorBackgroundAlertSuccess: '{colorBackgroundStatusSuccess}',
  colorBackgroundAlertWarning: '{colorBackgroundStatusWarning}',
-  colorBackgroundDialog: '{colorBackgroundStatusInfo}',
+  colorBackgroundDialog: '{colorBackgroundItemCard}',


Why do we refer to the item card here?

amanabiy deployed to dev-pages-react18 August 13, 2026 13:53 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react16 August 13, 2026 13:53 — with GitHub Actions View deployment

amanabiy deployed to allure-report August 13, 2026 13:59 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch from 4fb0a5e to f87b9bc Compare August 27, 2026 12:35

amanabiy deployed to dev-pages-react16 August 27, 2026 12:38 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react18 August 27, 2026 12:38 — with GitHub Actions View deployment

amanabiy deployed to allure-report August 27, 2026 12:43 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch 2 times, most recently from f87b9bc to 5f24213 Compare August 27, 2026 12:46

amanabiy deployed to dev-pages-react16 August 27, 2026 12:52 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react18 August 27, 2026 12:52 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react16 August 27, 2026 12:53 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react18 August 27, 2026 12:53 — with GitHub Actions View deployment

amanabiy temporarily deployed to allure-report August 27, 2026 12:58 — with GitHub Actions Inactive

amanabiy deployed to allure-report August 27, 2026 12:59 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch from 5f24213 to 2133113 Compare September 1, 2026 09:12

amanabiy deployed to dev-pages-react16 September 1, 2026 09:15 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react18 September 1, 2026 09:15 — with GitHub Actions View deployment

amanabiy deployed to allure-report September 1, 2026 09:21 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch from 2133113 to e62996c Compare September 1, 2026 11:42

amanabiy deployed to dev-pages-react18 September 1, 2026 11:45 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react16 September 1, 2026 11:45 — with GitHub Actions View deployment

amanabiy deployed to allure-report September 1, 2026 11:49 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch from e62996c to 4cd8d21 Compare September 1, 2026 11:54

amanabiy deployed to dev-pages-react18 September 1, 2026 11:56 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react16 September 1, 2026 11:56 — with GitHub Actions View deployment

amanabiy deployed to allure-report September 1, 2026 12:01 — with GitHub Actions View deployment

amanabiy force-pushed the dev-v3-amanabiy-dialog branch from 4cd8d21 to 97f9d8a Compare September 4, 2026 12:48

amanabiy deployed to dev-pages-react16 September 4, 2026 12:51 — with GitHub Actions View deployment

amanabiy deployed to dev-pages-react16 September 8, 2026 12:02 — with GitHub Actions View deployment

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread src/dialog/internal.tsx

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread src/__tests__/snapshot-tests/__snapshots__/style-docs.test.ts.snap Outdated

Comment thread src/dialog/interfaces.ts

Comment thread src/i18n/messages/all.en.json

Comment on lines +185 to +187

"dialog": {

"i18nStrings.dismissAriaLabel": "Close dialog"

},

feat: add dialog component

d4727e1

chore: clean up fix: add dismissAriaLabel

pan-kot reviewed Sep 9, 2026

View reviewed changes

Uh oh!

Conversation

amanabiy commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Correctness

Security

Testing

Uh oh!

codecov Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan-kot Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan-kot Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan-kot Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amanabiy commented Aug 13, 2026 •

edited

Loading

codecov Bot commented Aug 13, 2026 •

edited

Loading

pan-kot Sep 9, 2026 •

edited

Loading

pan-kot Sep 9, 2026 •

edited

Loading

pan-kot Sep 9, 2026 •

edited

Loading