fix(sp-alert-dialog): Make the divider color transparent for S2 theme #5747
Conversation
🦋 Changeset detectedLatest commit: f156687 The changes in this PR will be included in the next version bump. This PR includes changesets to release 84 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 |
📚 Branch Preview🔍 Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
TarunAdobe
changed the title
marissahuysentruyt
force-pushed
the
iuta/improve-alrt-dialog-spectrum-two
branch
from
1bfbcc4 to
f156687
Compare
Tachometer resultsChromealert-dialog permalinkbasic-test
Firefoxalert-dialog permalinkbasic-test
|
marissahuysentruyt
force-pushed
the
iuta/improve-alrt-dialog-spectrum-two
branch
from
2cb988b to
f156687
Compare
| .divider { | ||
| --spectrum-divider-background-color: var(--system-alert-dialog-divider-background-color); | ||
| --spectrum-divider-background-color-static-white: var(--spectrum-alert-dialog-divider-background-color-static-white); | ||
| --spectrum-divider-background-color-static-black: var(--spectrum-alert-dialog-divider-background-color-static-black); | ||
| } |
There was a problem hiding this comment.
In the JIRA ticket it is mentioned to remove the divider from the alert dialog only for S2 theme, to not update the DOM, but by using background color: The changes in alert dialog highlighted show a removed separator line. This may be accomplished in theme by changing the divider's color, but should not alter the DOM or APIs.
That's why I added these 3 tokens in the overrides file (i found them when I inspected the alert dialog component from the storybook) and used 3 new bridge tokens (because I don't want to make all the dividers from all SWC components transparent in S2, only the ones from the alert dialog).
If there is other solution better than this, please let me know, this is the only one I taught of and I am very open to suggestions, I want to do it the right way!
There was a problem hiding this comment.
I am fine with this approach for now but I want to make sure that we are future proofing this solutionaing. If the ask is to remove the alert dialog from S2 we can do this via the presentation layer rather that going the CSS way. Not a blocker for now but once we merge Gen2, we need to revisit this implementation.
Description
This PR improves the Alert dialog component in spectrum-two theme.
The divider color should be transparent for S2 theme.
Motivation and context
Fix the regressions found after changing from the express theme to the spectrum-two theme.
Related issue(s)
SWC-1077 (original)
CCEX-230905
Screenshots (if appropriate)
Spectrum 1
Spectrum 2
Express
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Descriptive Test Statement
Device review