feat(closebutton)!: migrate to S2 #2564

mdt2 · 2024-02-27T16:51:44Z

Description

Migrate close button to S2 per design spec.

Includes a refactor that removes all static-specific mods since they aren't needed:

| `--mod-closebutton-static-background-color-default` |
| `--mod-closebutton-static-background-color-down`    |
| `--mod-closebutton-static-background-color-focus`   |
| `--mod-closebutton-static-background-color-hover`   |

Consumers can use these existing background-color mods instead:

| `--mod-closebutton-background-color-default` |
| `--mod-closebutton-background-color-down`    |
| `--mod-closebutton-background-color-focus`   |
| `--mod-closebutton-background-color-hover`   |

Also removes --mod-closebutton-size since we'll be setting rounded border-radius with a single rounding token, to be formally implemented as part of S2 Corner Rounding.

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

Design validation @mdt2
In storybook:
- close button is the correct size at all t-shirt sizes
- close button matches the design spec

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

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

github-actions · 2024-02-27T16:54:42Z

File metrics

Summary

Total size: 3.90 MB^*
Total change (Δ): ⬇ 59.88 KB (-1.48%)
Table reports on changes to a package's main file. Other changes can be found in the collapsed "Details" below.

Package	Size	Δ
actiongroup	5.73 KB	⬇ 2.79 KB
buttongroup	1.38 KB	⬇ 0.53 KB
closebutton	11.19 KB	⬇ 4.95 KB
menu	41.11 KB	⬇ 0.26 KB
underlay	3.04 KB	⬇ 0.10 KB
tokens	196.12 KB	⬇ 3.61 KB

Details

actiongroup

File	Head	Base	Δ
index-base.css	5.73 KB	8.04 KB	⬇ 2.31 KB (-28.75%)
index-theme.css	0.59 KB	1.06 KB	⬇ 0.49 KB (-45.10%)
index-vars.css	5.73 KB	8.52 KB	⬇ 2.79 KB (-32.74%)
index.css	5.73 KB	8.52 KB	⬇ 2.79 KB (-32.74%)
mods.json	0.25 KB	0.40 KB	⬇ 0.15 KB (-37.47%)
themes/express.css	0.59 KB	0.89 KB	⬇ 0.29 KB (-32.96%)
themes/spectrum.css	0.59 KB	0.79 KB	⬇ 0.20 KB (-24.91%)

buttongroup

File	Head	Base	Δ
index-base.css	1.38 KB	1.90 KB	⬇ 0.53 KB (-27.32%)
index-vars.css	1.38 KB	1.90 KB	⬇ 0.53 KB (-27.32%)
index.css	1.38 KB	1.90 KB	⬇ 0.53 KB (-27.32%)
mods.json	0.15 KB	0.15 KB	-

closebutton

File	Head	Base	Δ
index-base.css	11.19 KB	15.50 KB	⬇ 4.31 KB (-27.79%)
index-vars.css	11.19 KB	16.14 KB	⬇ 4.95 KB (-30.67%)
index.css	11.19 KB	16.14 KB	⬇ 4.95 KB (-30.67%)
mods.json	1.05 KB	1.29 KB	⬇ 0.25 KB (-18.93%)

underlay

File	Head	Base	Δ
index-base.css	3.04 KB	3.14 KB	⬇ 0.10 KB (-3.02%)
index-vars.css	3.04 KB	3.14 KB	⬇ 0.10 KB (-3.02%)
index.css	3.04 KB	3.14 KB	⬇ 0.10 KB (-3.02%)
mods.json	0.48 KB	0.48 KB	-

tokens

File	Head	Base	Δ
css/dark-vars.css	35.34 KB	24.57 KB	⬆ 10.77 KB (43.83%)
css/express/custom-dark-vars.css	0.59 KB	0.59 KB	-
css/express/custom-darkest-vars.css	0.59 KB	0.59 KB	-
css/express/custom-large-vars.css	0.50 KB	0.50 KB	-
css/express/custom-light-vars.css	0.63 KB	0.63 KB	-
css/express/custom-medium-vars.css	0.48 KB	0.48 KB	-
css/express/custom-vars.css	0.04 KB	0.04 KB	-
css/global-vars.css	46.19 KB	38.23 KB	⬆ 7.97 KB (20.84%)
css/large-vars.css	27.35 KB	24.40 KB	⬆ 2.95 KB (12.07%)
css/light-vars.css	35.34 KB	24.55 KB	⬆ 10.79 KB (43.94%)
css/medium-vars.css	27.27 KB	24.33 KB	⬆ 2.94 KB (12.07%)
css/spectrum/custom-dark-vars.css	3.40 KB	3.40 KB	-
css/spectrum/custom-darkest-vars.css	3.40 KB	3.40 KB	-
css/spectrum/custom-large-vars.css	4.94 KB	4.94 KB	-
css/spectrum/custom-light-vars.css	3.40 KB	3.40 KB	-
css/spectrum/custom-medium-vars.css	5.18 KB	5.18 KB	-
css/spectrum/custom-vars.css	2.04 KB	2.04 KB	-
index.css	196.12 KB	199.73 KB	⬇ 3.61 KB (-1.81%)

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

github-actions · 2024-02-27T16:54:53Z

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

mdt2 · 2024-02-27T20:36:18Z

components/closebutton/metadata/closebutton.yml

-        </div>
-      </div>
-  - id: closebutton-largeicon
-    name: Icon Size - Large


Per the changelog, icon size goes away in S2

components/closebutton/metadata/closebutton.yml

jawinn

This is looking pretty good so far.

One thing I noticed is that it looks like the "Cross icon (static white)" and "Cross icon (static black)" tokens on the Figma may still need to be set within the .spectrum-CloseButton--staticWhite and .spectrum-CloseButton--staticBlack classes. I didn't see any CSS that had the transparent-white-800 and transparent-white-900 for example.

The other issue may be build related, but I'm not seeing the hover background color on the docs page. It looks fine in Storybook.

Cleanup of "Hardcoded" tokens in CSS
The other suggestion I had when looking at the previously existing CSS, is that we can probably remove all of these variables:

--spectrum-closebutton-size-300
--spectrum-closebutton-size-400
--spectrum-closebutton-size-500
--spectrum-closebutton-size-600
--spectrum-closebutton-size

All they seem to be used for is setting the border-radius to rounded. We can handle that with a single 9999px rounding token, like the one being proposed in PR #2559 . Maybe for now use a temporary token to be replaced with the --spectrum-corner-radius-1000-full when it is available?

components/closebutton/metadata/closebutton.yml

components/closebutton/index.css

components/closebutton/metadata/mods.md

jawinn

Those updates look good! That resolves my previous comments about the icon color and the hardcoded tokens for corner rounding.

Two last questions / suggestions:

Should the express.css and spectrum.css files be deleted completely, now that they're no longer required in the build process?
Could your notes about the mod change in the PR be added to the migration guide? Since consumers could be relying on those old mod properties. Here's a general idea for the formatting of the S2 migration guide note, if that's helpful.

BREAKING CHANGE: migrates Close Button to Spectrum 2 Additionally: * test: increase chromatic coverage * fix(closebutton): pass staticColor as arg for SB display * chore(closebutton): remove themes dir * docs(closebutton): adds s2 migration notes * chore(closebutton): specify s2 tokens release for dependency --------- Co-authored-by: Patrick Fulton <pfulton@adobe.com>

mdt2 requested review from pfulton, jawinn, castastrophe and rise-erpelding February 27, 2024 20:01

mdt2 force-pushed the mdt2/css-694-close-button branch from 09c0cb6 to cf8ba57 Compare February 27, 2024 20:35

mdt2 commented Feb 27, 2024

View reviewed changes

mdt2 force-pushed the mdt2/css-694-close-button branch from cf8ba57 to e4ebaa1 Compare February 27, 2024 20:57

rise-erpelding reviewed Feb 27, 2024

View reviewed changes

components/closebutton/metadata/closebutton.yml Outdated Show resolved Hide resolved

pfulton added the run_vrt For use on PRs looking to kick off VRT label Feb 28, 2024

mdt2 added the wip This is a work in progress, don't judge. label Feb 28, 2024

castastrophe added the S2 Spectrum 2 label Feb 28, 2024

jawinn requested changes Feb 28, 2024

View reviewed changes

components/closebutton/metadata/closebutton.yml Outdated Show resolved Hide resolved

mdt2 force-pushed the mdt2/css-694-close-button branch from a7281fd to 165db1c Compare February 28, 2024 22:25

mdt2 commented Feb 28, 2024

View reviewed changes

components/closebutton/index.css Show resolved Hide resolved

mdt2 force-pushed the mdt2/css-694-close-button branch from 165db1c to bd28ffe Compare February 28, 2024 22:34

mdt2 commented Feb 28, 2024

View reviewed changes

components/closebutton/metadata/mods.md Show resolved Hide resolved

feat(closebutton)!: migrate to S2

fde3cbf

mdt2 force-pushed the mdt2/css-694-close-button branch from bd28ffe to fde3cbf Compare February 29, 2024 22:07

mdt2 and others added 2 commits March 1, 2024 15:48

test: increase chromatic coverage

8900e54

fix(closebutton): pass staticColor as arg for SB display

f5c5d93

jawinn requested changes Mar 5, 2024

View reviewed changes

pfulton added 3 commits March 6, 2024 10:44

chore(closebutton): remove themes dir

d7b1e00

docs(closebutton): adds s2 migration notes

1712448

chore(closebutton): specify s2 tokens release for dependency

3c23c41

jawinn approved these changes Mar 6, 2024

View reviewed changes

pfulton merged commit 9dea2aa into spectrum-two Mar 7, 2024
12 of 13 checks passed

pfulton deleted the mdt2/css-694-close-button branch March 7, 2024 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(closebutton)!: migrate to S2 #2564

feat(closebutton)!: migrate to S2 #2564

mdt2 commented Feb 27, 2024 •

edited

github-actions bot commented Feb 27, 2024 •

edited

actiongroup

buttongroup

closebutton

menu

underlay

tokens

github-actions bot commented Feb 27, 2024 •

edited

mdt2 Feb 27, 2024

jawinn left a comment •

edited

jawinn left a comment •

edited

feat(closebutton)!: migrate to S2 #2564

feat(closebutton)!: migrate to S2 #2564

Conversation

mdt2 commented Feb 27, 2024 • edited

Description

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

github-actions bot commented Feb 27, 2024 • edited

File metrics

Summary

actiongroup

buttongroup

closebutton

menu

underlay

tokens

github-actions bot commented Feb 27, 2024 • edited

mdt2 Feb 27, 2024

Choose a reason for hiding this comment

jawinn left a comment • edited

Choose a reason for hiding this comment

jawinn left a comment • edited

Choose a reason for hiding this comment

mdt2 commented Feb 27, 2024 •

edited

github-actions bot commented Feb 27, 2024 •

edited

github-actions bot commented Feb 27, 2024 •

edited

jawinn left a comment •

edited

jawinn left a comment •

edited