fix(modal): change modal background color to transparent#3604
Conversation
🦋 Changeset detectedLatest commit: 7c3f7e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
File metricsSummaryTotal size: 1.37 MB*
modal
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
| options: ["default", "fullscreen", "fullscreenTakeover"], | ||
| control: "select", | ||
| }, | ||
| isOpen, |
There was a problem hiding this comment.
An extra isOpen must have been brought over by mistake during a rebase or something.
|
🚀 Deployed on https://pr-3604--spectrum-css.netlify.app |
| // 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: { |
There was a problem hiding this comment.
Dialog has a defined background color, so these custom background colors are no longer needed.
| }; | ||
|
|
||
| /** | ||
| * 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. |
There was a problem hiding this comment.
I'm open to suggestions on how to describe this.
5ad053e to
3bc4281
Compare
|
TODO: run vrts |
3bc4281 to
7f70ede
Compare
7f70ede to
35966fa
Compare
castastrophe
left a comment
There was a problem hiding this comment.
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.
35966fa to
b9f2cd1
Compare
- 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
b9f2cd1 to
7c3f7e0
Compare
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-colorto fallback totransparentinstead ofbackground-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 thecustomStylesarg from the dialog template that was setting the--mod-modal-background-colorto 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: transparentis acceptable, while the mod continues to offer flexibility to our consumers if they need it..spectrum-Modalelement. The background color should resolve totransparent.--mod-modal-background-colorto.spectrum-Modaland 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..spectrum-Modalwrapper. Set--mod-modal-background-colorto your color choice once more if it's not still defined.--spectrum-modal-background-colorresolves totransparent, the anti-aliasing bleed is not present. ✅.spectrum-Dialogelements, the background-color should bebackground-layer-2-color(which resolves togray-25/gray-75depending on light or dark mode), which is defined as the background color for S2 dialogs..spectrum-Modalcontaining element for alert dialog. In your browser, set--mod-modal-background-colorto any color you wish to verify the mod is working as expected.Regression testing
Validate:
Screenshots
Before 🚫

modal (where the background color is set on
.spectrum-Modaldialog 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