Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dialog): s2 takeover dialog migration #3347

Merged

Conversation

marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented Oct 29, 2024

Description

This implements the design updates and new tokens for the fullscreen and fullscreen takeover dialogs in S2. 🎉 The majority of the migration has been done in the default, standard dialog (from #2860). The effected stories are called Takeover Dialog in Figma. New tokens were utilized to match the design specs. Fullscreen and fullscreenTakeover dialogs do not support additional footer content, checkboxes, or hero images. They are also not dismissible.

Documentation has been updated with info on the takeover dialog variants and the divider story was removed since dividers are no longer supported in dialog components in S2.

Takeover dialogs in Figma support swapping out text for other components in the header content and body areas. The template for dialogs changed to reflect that, utilizing renderContent. Additionally, some class names were corrected to follow our typical naming convention (.spectrum-Dialog-header-content > .spectrum-Dialog-headerContent and
.spectrum-Dialog-footer-content > .spectrum-Dialog-footerContent).

New mod properties should reflect the "takeover dialog" language:

New Mods
--mod-takeover-dialog-grid-spacing
--mod-takeover-dialog-spacing-header-content-gap
--mod-takeover-dialog-title-font-size

New modal custom property
--spectrum-modal-takeover-window-to-edge

New aliased tokens were created for mobile value changes:
For .spectrum--large:
--spectrum-takeover-dialog-spacing-grid-padding:var(--spectrum-spacing-400);
--spectrum-takeover-dialog-spacing-header-gap:var(--spectrum-spacing-300);
--spectrum-takeover-dialog-spacing-title-to-body:var(--spectrum-spacing-400);

For .spectrum--medium:
--spectrum-takeover-dialog-spacing-grid-padding:var(--spectrum-spacing-500);
--spectrum-takeover-dialog-spacing-header-gap:var(--spectrum-spacing-400);
--spectrum-takeover-dialog-spacing-title-to-body:var(--spectrum-spacing-500);

Designs

S2 Takeover Dialog Token specs
S2 / Desktop Takeover dialog

Jira

This work was captured as "follow up" work in CSS-829
(The bulk of the dialog migration occurred in CSS-714)

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  • Pull down the branch to run locally or use the deploy preview.
  • Visit the dialog storybook. Verify both fullscreen and fullscreenTakeover stories appear on the docs page.
  • Visit the fullscreen dialog and fullscreenTakeover dialog story pages.
  • The fullscreen & fullscreen takeover dialog stories matches the s2 takeover dialog specs.
  • The dialog no longer supports the Divider component, so any JS or CSS referencing the dialog's divider has been removed from the stories and index.css files.
  • The takeover dialog variants should not have a isDismissible control since they don't support the close button.
  • Verify that the fullscreen & fullscreenTakeover stories link to the Takeover Dialog Figma file within their respective Design add-on panels.
  • Verify the .spectrum-Modal--fullscreen utilizes the updated window-to-edge value (spacing-600 40px), corresponding to the .spectrum-Dialog--fullscreen story.
  • Verify the button group is keyboard accessible and the focus rings do not get cut off.
  • Verify the .spectrum-Modal--fullscreenTakeover utilizes the updated window-to-edge value (0), corresponding to the .spectrum-Dialog--fullscreenTakeover story.
  • Create various combinations of the takeover dialogs to verify combos are considered in the CSS:
    • mobile vs desktop
    • light vs dark mode
  • Feel free to double check the standard dialog controls and options as well. hasHeroImage, isDismissible, hasFooter, are not supported in takeover dialogs, so their controls should only appear for the standard dialog.
  • You may run into the error "Rendered fewer hooks than expected." when using certain controls. Refreshing the window will remove the error.
  • Verify new tokens are used:
    • takeover-dialog-width
    • takeover-dialog-height
    • window-to-edge
  • Verify updated tokens are used:
    • corner-radius-extra-large-default
    • takeover-dialog-spacing-grid-padding
    • takeover-dialog-spacing-header-gap
    • takeover-dialog-spacing-title-to-body
    • background-layer-2-color (in the designs, this is accidentally misnamed as background-color-layer2)
  • Chromatic coverage (using dialog.test.js file) continues to maintain the fullscreen and fullscreenTakeover test templates, and do not have additional variations/test scenarios.

Additional validation

  • If running the branch locally, feel free to test the renderContent function for the header controls for the takeover dialogs. For instance, put a different component, like help text or breadcrumbs (don't be alarmed if breadcrumb icons do not render- this is a known bug).
  • Verify that new header component is centered in the .spectrum-Dialog-headerContentWrapper element.
  • If running the branch locally, feel free to test other components for the content controls for the takeover dialogs. For instance, import a different component, like swatch group, or form (don't be alarmed if form styles do not render- this is a known bug).
  • Verify that new content component renders in the .spectrum-Dialog-content element.
  • Revert any changes you make 😊

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

Copy link

changeset-bot bot commented Oct 29, 2024

🦋 Changeset detected

Latest commit: 1c491ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@spectrum-css/dialog Major
@spectrum-css/modal Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@marissahuysentruyt marissahuysentruyt added the wip This is a work in progress, don't judge. label Oct 29, 2024
Copy link
Contributor

github-actions bot commented Oct 29, 2024

🚀 Deployed on https://pr-3347--spectrum-css.netlify.app

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from 37d987f to b86d610 Compare October 29, 2024 14:13
@marissahuysentruyt marissahuysentruyt added the do not merge A flag for a branch indicating it should not be merged. label Oct 29, 2024
Copy link
Contributor

github-actions bot commented Oct 29, 2024

File metrics

Summary

Total size: 2.23 MB*
Total change (Δ): 🔴 ⬆ 3.84 KB (0.17%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
dialog 16.36 KB 🔴 ⬆ 1.37 KB
modal 4.51 KB 🔴 ⬆ 0.06 KB
tokens 241.49 KB 🔴 ⬆ 0.47 KB

Details

dialog

Filename Head Compared to base
index.css 16.36 KB 🔴 ⬆ 1.37 KB (9.10%)
metadata.json 6.58 KB 🔴 ⬆ 0.94 KB (16.24%)

modal

Filename Head Compared to base
index.css 4.51 KB 🔴 ⬆ 0.06 KB (1.25%)
metadata.json 1.81 KB 🔴 ⬆ 0.12 KB (6.81%)

tokens

Filename Head Compared to base
css/dark-vars.css 46.32 KB -
css/global-vars.css 69.69 KB -
css/index.css 241.49 KB 🔴 ⬆ 0.47 KB (0.19%)
css/large-vars.css 40.75 KB 🔴 ⬆ 0.24 KB (0.57%)
css/light-vars.css 46.29 KB -
css/medium-vars.css 40.88 KB 🔴 ⬆ 0.24 KB (0.57%)
index.css 241.49 KB 🔴 ⬆ 0.47 KB (0.19%)
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@marissahuysentruyt marissahuysentruyt changed the base branch from spectrum-two to marissahuysentruyt/css-714-s2-dialog October 29, 2024 18:19
@marissahuysentruyt marissahuysentruyt added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Oct 30, 2024
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-dialog branch from 5c3c954 to 179f3f8 Compare October 30, 2024 21:41
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from b86d610 to 3b5b527 Compare October 31, 2024 16:36
Comment on lines +2 to +3
"@spectrum-css/dialog": major
"@spectrum-css/modal": minor
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we agree with these version bumps? Is it a concern that the dialog component will have undergone 2 major version bumps for S2? I think the changes would be considered breaking changes. And really, the only reason for a second major version is because we split the standard dialog work from this takeover dialog work. Any thoughts there?

Same for modal- is it a minor version bump?

Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@castastrophe any reservations with the version bumps here? I wasn't sure if 2 major bumps for dialog (we had a major version change for #2860, and I'm proposing a secondary major version bump) was an issue or weird or something.

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from 3b5b527 to 4c4d044 Compare October 31, 2024 16:54
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-dialog branch from 88db1a4 to 4168200 Compare November 4, 2024 14:26
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch 3 times, most recently from 8b79f9a to f63164c Compare November 8, 2024 21:53
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from f63164c to bff05df Compare November 19, 2024 17:48
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-dialog branch from f1e6597 to 70de6c8 Compare November 21, 2024 21:38
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from bff05df to bec02e7 Compare December 2, 2024 19:34
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-dialog branch 2 times, most recently from b71d0c6 to acde6e5 Compare December 6, 2024 02:59
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from bec02e7 to bec4b1c Compare December 6, 2024 03:48
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-dialog branch from acde6e5 to 531a17b Compare December 6, 2024 15:04
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch 2 times, most recently from a4619d1 to 65d15e2 Compare December 9, 2024 22:02
Base automatically changed from marissahuysentruyt/css-714-s2-dialog to spectrum-two December 9, 2024 22:36
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch 2 times, most recently from 2bcf5a9 to e3f01b8 Compare December 10, 2024 16:51
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from ccc13e7 to 90a7253 Compare January 16, 2025 18:11
@castastrophe castastrophe force-pushed the spectrum-two branch 3 times, most recently from 46ae687 to 8d65de0 Compare January 21, 2025 15:55
Copy link
Collaborator

@aramos-adobe aramos-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only feature I was looking for has been added! Great work @marissahuysentruyt

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from 90a7253 to 7d12a13 Compare January 22, 2025 17:14
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch 2 times, most recently from 7bad7ad to e3e1efb Compare January 22, 2025 18:52
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch 2 times, most recently from 822a5ce to 5ccfca9 Compare January 29, 2025 17:43
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles

- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-714-s2-takeover-dialog branch from 5ccfca9 to 1c491ed Compare January 30, 2025 17:25
Copy link
Collaborator

@5t3ph 5t3ph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@marissahuysentruyt marissahuysentruyt merged commit c297d1c into spectrum-two Jan 30, 2025
12 checks passed
@marissahuysentruyt marissahuysentruyt deleted the marissahuysentruyt/css-714-s2-takeover-dialog branch January 30, 2025 17:54
jawinn pushed a commit that referenced this pull request Feb 3, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 5, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
pfulton pushed a commit that referenced this pull request Feb 6, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 7, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 7, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 7, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 11, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 24, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 24, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
castastrophe pushed a commit that referenced this pull request Feb 25, 2025
* feat(dialog): adds new takeover dialog styles
- new tokens and takeover dialog styles were implemented in index.css
- rebuild tokens/metadata

* docs(dialog): updates dialog template and stories
- adds layout: "fullscreen" to render dialogs in their canvases, instead
of off the canvas
- uses getRandomId and renderContent functions to render header and
footer contents
- implements "slotted" components for the new takeover dialog feature
- removes hasDivider since s2 dialogs don't support a divider
- updates Figma links for fullscreen and fullscreenTakeover stories
- updates some class names to better follow naming conventions

* chore(dialog): clean up dialog tests
- adds hasHeroImage arg
- adds background and margin customStyles for fullscreen dialog in
Chromatic only (so we can see the rounded corners better)

fix(modal): update modal with takeover dialog styles
- builds metadata
- adds tokens for modal that complement new takeover dialog styles/tokens

chore(dialog,modal): create changeset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review S2 Spectrum 2 size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants