fix(modal): change modal background color to transparent #3604

marissahuysentruyt · 2025-03-05T19:31:00Z

Description

Previously, the modal was the component that set the background color for its children- the S1 modal has a background-color of gray-50, and the S1 dialog didn't have a background-color defined. For S2, more components have their own background-colors defined, ie the S2 dialog has a background-color of gray-50.

Because both the modal and the child component had defined background colors, there was some bleeding, especially in the corners, where you could see the modal background color:

Screen.Recording.2025-03-05.at.2.07.25.PM.mov

This PR should update --spectrum-modal-background-color to fallback to transparent instead of background-color-layer-2. Additionally, this update made it so that we could also remove some dialog code that was trying to combat this. I deleted the customStyles arg from the dialog template that was setting the --mod-modal-background-color to transparent anyways (a hotfix implemented in #2860: https://github.com/adobe/spectrum-css/pull/2860/files#diff-dd11973c8d7472852e8c9bb250b44fe9ecdc299b0abbd0c9a2fae8b303840662R186), and removed the altered background color from dialog's test template.

Jira/Specs

CSS-1057
S2 dialog token specs

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

We'll be testing that the mod works in various places and components, to validate that the initial --spectrum-modal-background-color: transparent is acceptable, while the mod continues to offer flexibility to our consumers if they need it.

Pull down the branch to run locally or visit the deploy preview.
On the modal docs page, inspect the .spectrum-Modal element. The background color should resolve to transparent.
In the inspector, add --mod-modal-background-color to .spectrum-Modal and set it to whatever color you'd like. Confirm that the background of that element changes to your chosen color. The element should be more "visible" with rounded corners.
Visit the dialog docs page. Visually, verify that nothing on this page looks broken or off. This should look identical.
Inspect the .spectrum-Modal wrapper. Set --mod-modal-background-color to your color choice once more if it's not still defined.
Visually inspect the rounded corners of the dialog. When the mod for the modal background is set, you should see a sliver of the background color.
- sometimes it's easier to see this in dark mode, zooming in on the component pretty far, and setting the modal mod to a really bright color.

In the inspector, toggle the mod property for the modal background color on and off. Verify that when --spectrum-modal-background-color resolves to transparent, the anti-aliasing bleed is not present. ✅
Visit the testing preview for dialog. When inspecting any of the .spectrum-Dialog elements, the background-color should be background-layer-2-color (which resolves to gray-25/gray-75 depending on light or dark mode), which is defined as the background color for S2 dialogs.
- The testing preview for the fullscreen and fullscreenTakeover dialogs can be validated in the same way.
Visit the tray docs page. Verify no regressions occurred. This should look identical.
Visit the alert dialog docs page. This is the only component that "looks" broken because of this modal background color change.
Inspect the .spectrum-Modal containing element for alert dialog. In your browser, set --mod-modal-background-color to any color you wish to verify the mod is working as expected.
- NOTE: in subsequent work, the alert dialog will receive its own background color token definition.

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

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

Before 🚫
modal (where the background color is set on .spectrum-Modal

dialog tests (where we were compensating for dialog not having its own background color in S1)

alert dialog (where the background color is set on .spectrum-Modal)

After ✅
modal (because we're only using typography here, no background color is set. See the documentation added for an explanation for implementations)

dialog tests (with the removal of custom background styles for the dialogs)

alert dialog (although this looks broken now, after the alert dialog migrates to S2, it will also have its own background color)

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. ✨

changeset-bot · 2025-03-05T19:31:04Z

🦋 Changeset detected

Latest commit: 7c3f7e0

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

This PR includes changesets to release 4 packages

Name	Type
@spectrum-css/modal	Patch
@spectrum-css/alertdialog	Patch
@spectrum-css/dialog	Patch
@spectrum-css/tray	Patch

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

github-actions · 2025-03-05T19:35:27Z

File metrics

Summary

Total size: 1.37 MB^*

Package	Size	Minified	Gzipped
modal	4.50 KB	4.24 KB	1.19 KB

modal

Filename	Head	Minified	Gzipped	Compared to base
index.css	4.50 KB	4.24 KB	1.19 KB	🟢 ⬇ 0.01 KB
metadata.json	1.78 KB	-	-	🟢 ⬇ 0.03 KB

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

marissahuysentruyt · 2025-03-05T19:36:15Z

components/dialog/stories/dialog.stories.js

@@ -90,7 +90,6 @@ export default {
 			options: ["default", "fullscreen", "fullscreenTakeover"],
 			control: "select",
 		},
-		isOpen,


An extra isOpen must have been brought over by mistake during a rebase or something.

github-actions · 2025-03-05T19:36:20Z

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

marissahuysentruyt · 2025-03-05T21:02:23Z

components/dialog/stories/dialog.test.js

@@ -16,10 +16,6 @@ export const DialogGroup = Variants({
 			// TODO: The dialog's heading arg is getting passed as the "Sizing" heading arg (instead of the
 			// TODO: word "Sizing"). We should be able to remove this arg once that no longers happens.
 			heading: showTestingGrid ? "Lorem ipsum dolor sit amet, consectetur adipiscing elit" : args.heading,
-			customStyles: {


Dialog has a defined background color, so these custom background colors are no longer needed.

marissahuysentruyt · 2025-03-05T21:03:18Z

components/modal/stories/modal.stories.js

@@ -57,6 +57,9 @@ export default {
 	],
 };

+/**
+ * The default modal does not include a background color itself, other than `transparent`. If implementations are in need of a background color, and one is not supported within the modal's child component (for example, the dialog defines its own background color), `--mod-modal-background-color` may be set on the `.spectrum-Modal` class.


I'm open to suggestions on how to describe this.

marissahuysentruyt · 2025-03-05T21:24:10Z

TODO: run vrts

castastrophe

This update looks great and I love how much simpler it is to render this now for Storybook. Great changeset details and thanks for adding comments throughout to help clarify changes! This looks good to go.

- adds documentation to modal docs page - updates metadata.json

- removes the customStyles object that was setting --mod-modal- background-color in the dialog template - removes the custom wrapper styles in the dialog tests that were intending to set the dialog apart from the modal - remove a duplicate isOpen control in dialog.stories.js

marissahuysentruyt self-assigned this Mar 5, 2025

marissahuysentruyt commented Mar 5, 2025

View reviewed changes

marissahuysentruyt added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. S2 Spectrum 2 labels Mar 5, 2025

marissahuysentruyt commented Mar 5, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/css-1057-transparent-modal-background branch from 5ad053e to 3bc4281 Compare March 5, 2025 21:04

marissahuysentruyt marked this pull request as ready for review March 5, 2025 21:25

marissahuysentruyt force-pushed the marissahuysentruyt/css-1057-transparent-modal-background branch from 3bc4281 to 7f70ede Compare March 5, 2025 21:29

marissahuysentruyt requested review from jawinn, castastrophe, rise-erpelding, cdransf, 5t3ph and aramos-adobe March 5, 2025 21:32

rise-erpelding approved these changes Mar 6, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/css-1057-transparent-modal-background branch from 7f70ede to 35966fa Compare March 7, 2025 15:37

castastrophe approved these changes Mar 11, 2025

View reviewed changes

castastrophe added the run_vrt For use on PRs looking to kick off VRT label Mar 11, 2025

castastrophe force-pushed the marissahuysentruyt/css-1057-transparent-modal-background branch from 35966fa to b9f2cd1 Compare March 11, 2025 15:13

marissahuysentruyt added 4 commits March 11, 2025 12:01

refactor(modal): update modal background color to transparent

65ae653

- adds documentation to modal docs page - updates metadata.json

chore(modal): create changeset

a69dc5f

chore: rebase and update metadata.json

7c3f7e0

marissahuysentruyt force-pushed the marissahuysentruyt/css-1057-transparent-modal-background branch from b9f2cd1 to 7c3f7e0 Compare March 11, 2025 16:05

marissahuysentruyt merged commit cb1b957 into spectrum-two Mar 11, 2025
13 checks passed

marissahuysentruyt deleted the marissahuysentruyt/css-1057-transparent-modal-background branch March 11, 2025 16:13

github-actions bot mentioned this pull request Mar 12, 2025

chore: release (next) #3620

Merged

castastrophe mentioned this pull request Apr 15, 2025

feat(tokens,card): update to @adobe/spectrum-tokens@13.3.0 #3669

Merged

12 tasks

This was referenced May 16, 2025

chore(swatch,menu,swatchgroup): fix reverted metadata assets #3752

Merged

chore: merge latest updates from main #3777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(modal): change modal background color to transparent #3604

fix(modal): change modal background color to transparent #3604

Uh oh!

marissahuysentruyt commented Mar 5, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Mar 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 5, 2025 •

edited

Loading

Uh oh!

marissahuysentruyt Mar 5, 2025

Uh oh!

github-actions bot commented Mar 5, 2025 •

edited

Loading

Uh oh!

marissahuysentruyt Mar 5, 2025

Uh oh!

marissahuysentruyt Mar 5, 2025

Uh oh!

marissahuysentruyt commented Mar 5, 2025

Uh oh!

castastrophe left a comment

Uh oh!

Uh oh!

Uh oh!

fix(modal): change modal background color to transparent #3604

fix(modal): change modal background color to transparent #3604

Uh oh!

Conversation

marissahuysentruyt commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Jira/Specs

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

Uh oh!

changeset-bot bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

modal

Uh oh!

marissahuysentruyt Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marissahuysentruyt Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt commented Mar 5, 2025

Uh oh!

castastrophe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marissahuysentruyt commented Mar 5, 2025 •

edited

Loading

changeset-bot bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading