spectrum-web-components
spectrum-web-components copied to clipboard
refactor(badge): use core tokens
This updates the Badge component to use core tokens
Uses beta release. Hold for full release.
Description
Related issue(s)
Motivation and context
How has this been tested?
- [ ] Test case 1
- Go here
- Do this
- [ ] Test case 2
- Go here
- Do this
Screenshots (if appropriate)
Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)
Checklist
- [ ] I have signed the Adobe Open Source CLA.
- [ ] My code follows the code style of this project.
- [ ] If my change required a change to the documentation, I have updated the documentation in this pull request.
- [ ] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
- [ ] I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.
Branch Preview
Visual regression test results
When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
- High Contrast Mode | Medium | LTR
- Classic | Lightest | Medium | LTR
- Classic | Lightest | Medium | RTL
- Classic | Lightest | Large | LTR
- Classic | Lightest | Large | RTL
- Classic | Light | Medium | LTR
- Classic | Light | Medium | RTL
- Classic | Light | Large | LTR
- Classic | Light | Large | RTL
- Classic | Dark | Medium | LTR
- Classic | Dark | Medium | RTL
- Classic | Dark | Large | LTR
- Classic | Dark | Large | RTL
- Classic | Darkest | Medium | LTR
- Classic | Darkest | Medium | RTL
- Classic | Darkest | Large | LTR
- Classic | Darkest | Large | RTL
- Express | Lightest | Medium | LTR
- Express | Lightest | Medium | RTL
- Express | Lightest | Large | LTR
- Express | Lightest | Large | RTL
- Express | Light | Medium | LTR
- Express | Light | Medium | RTL
- Express | Light | Large | LTR
- Express | Light | Large | RTL
- Express | Dark | Medium | LTR
- Express | Dark | Medium | RTL
- Express | Dark | Large | LTR
- Express | Dark | Large | RTL
- Express | Darkest | Medium | LTR
- Express | Darkest | Medium | RTL
- Express | Darkest | Large | LTR
- Express | Darkest | Large | RTL
It appears that all the failures are likely due to changed markup that adds a "spectrum-Badge-label" element to control the spacing required by the addition of the icon variant.
The markup of web-components will need to be updated to reflect the correct styles. Please advise if I should make any changes.
@Westbrook
There are a lot of DOM changes here, yes. @pfulton this makes the Badge even more like the Action Button, I know we don't usually share code this way, but should we share code here?
With the differences, would it be better for us to take this one on, or would it make sense to pair a little on it or?
There are a lot of DOM changes here, yes. @pfulton this makes the Badge even more like the Action Button, I know we don't usually share code this way, but should we share code here?
π¬ I'm not sure! My gut reaction is no, don't share - I'm more worried about how consumers would understand the differences between the two than I am about the code side of it. Do you think it'd be difficult to advise folks on what to use and when (especially if there would come a time when folks would need to customize either of these?).
That is, unless you're talking about having some "base" level component that both of these would inherit from. Maybe then I could see some sharing.
With the differences, would it be better for us to take this one on, or would it make sense to pair a little on it or?
Yeah, I think given our workload, we might need to rely on you all for the integration work. We're happy to support/fix any bugs with the CSS, but for this one, we might need y'all to carry it across the finish line.
Tachometer results
Chrome
action-bar permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 244 kB | 32.83ms - 33.13ms | - | unsure π -1% - +0% -0.36ms - +0.09ms |
| branch | 244 kB | 32.96ms - 33.28ms | unsure π -0% - +1% -0.09ms - +0.36ms |
- |
action-menu permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 612 kB | 219.01ms - 220.84ms | - | unsure π -1% - +0% -2.70ms - +0.83ms |
| branch | 610 kB | 219.35ms - 222.38ms | unsure π -0% - +1% -0.83ms - +2.70ms |
- |
badge permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 275 kB | 40.61ms - 40.87ms | - | faster β 13% - 13% 5.89ms - 6.30ms |
| branch | 271 kB | 46.68ms - 46.99ms | slower β 14% - 15% 5.89ms - 6.30ms |
- |
card permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 392 kB | 86.71ms - 88.10ms | - | unsure π -1% - +2% -0.71ms - +1.36ms |
| branch | 391 kB | 86.31ms - 87.84ms | unsure π -2% - +1% -1.36ms - +0.71ms |
- |
illustrated-message permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 293 kB | 40.64ms - 40.98ms | - | unsure π -0% - +1% -0.13ms - +0.35ms |
| branch | 293 kB | 40.53ms - 40.87ms | unsure π -1% - +0% -0.35ms - +0.13ms |
- |
menu permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 311 kB | 247.18ms - 249.16ms | - | unsure π -1% - +0% -2.99ms - +0.34ms |
| branch | 311 kB | 248.16ms - 250.84ms | unsure π -0% - +1% -0.34ms - +2.99ms |
- |
overlay permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 348 kB | 75.02ms - 75.45ms | - | unsure π -1% - +0% -0.63ms - +0.37ms |
| branch | 348 kB | 74.91ms - 75.82ms | unsure π -0% - +1% -0.37ms - +0.63ms |
- |
picker permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 456 kB | 797.72ms - 806.74ms | - | unsure π -3% - +0% -22.13ms - +3.13ms |
| branch | 454 kB | 799.93ms - 823.53ms | unsure π -0% - +3% -3.13ms - +22.13ms |
- |
popover permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 241 kB | 30.12ms - 30.42ms | - | unsure π -0% - +1% -0.13ms - +0.23ms |
| branch | 241 kB | 30.13ms - 30.31ms | unsure π -1% - +0% -0.23ms - +0.13ms |
- |
slider permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 341 kB | 148.01ms - 149.06ms | - | unsure π -1% - +1% -1.09ms - +0.99ms |
| branch | 340 kB | 147.69ms - 149.49ms | unsure π -1% - +1% -0.99ms - +1.09ms |
- |
split-button permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 548 kB | 2028.15ms - 2031.03ms | - | unsure π -0% - +0% -1.90ms - +2.28ms |
| branch | 547 kB | 2027.88ms - 2030.93ms | unsure π -0% - +0% -2.28ms - +1.90ms |
- |
tooltip permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 248 kB | 30.22ms - 30.43ms | - | unsure π -1% - +0% -0.18ms - +0.13ms |
| branch | 247 kB | 30.23ms - 30.46ms | unsure π -0% - +1% -0.13ms - +0.18ms |
- |
Firefox
action-bar permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 244 kB | 113.51ms - 127.05ms | - | unsure π -5% - +11% -5.89ms - +13.17ms |
| branch | 244 kB | 109.94ms - 123.34ms | unsure π -11% - +5% -13.17ms - +5.89ms |
- |
action-menu permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 612 kB | 422.57ms - 437.07ms | - | unsure π -3% - +2% -12.67ms - +8.99ms |
| branch | 610 kB | 423.62ms - 439.70ms | unsure π -2% - +3% -8.99ms - +12.67ms |
- |
badge permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 275 kB | 118.32ms - 129.84ms | - | unsure π -9% - +1% -12.32ms - +1.68ms |
| branch | 271 kB | 125.43ms - 133.37ms | unsure π -2% - +10% -1.68ms - +12.32ms |
- |
card permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 392 kB | 185.04ms - 200.72ms | - | unsure π -7% - +5% -12.71ms - +9.27ms |
| branch | 391 kB | 186.90ms - 202.30ms | unsure π -5% - +7% -9.27ms - +12.71ms |
- |
illustrated-message permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 293 kB | 93.95ms - 103.29ms | - | unsure π -11% - +4% -11.39ms - +3.99ms |
| branch | 293 kB | 96.21ms - 108.43ms | unsure π -4% - +12% -3.99ms - +11.39ms |
- |
menu permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 311 kB | 542.29ms - 555.59ms | - | faster β 1% - 6% 7.56ms - 33.04ms |
| branch | 311 kB | 558.38ms - 580.10ms | slower β 1% - 6% 7.56ms - 33.04ms |
- |
overlay permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 348 kB | 168.53ms - 177.19ms | - | unsure π -8% - +1% -14.64ms - +1.28ms |
| branch | 348 kB | 172.86ms - 186.22ms | unsure π -1% - +9% -1.28ms - +14.64ms |
- |
picker permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 456 kB | 1171.32ms - 1184.72ms | - | unsure π -1% - +1% -10.87ms - +7.47ms |
| branch | 454 kB | 1173.45ms - 1185.99ms | unsure π -1% - +1% -7.47ms - +10.87ms |
- |
popover permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 241 kB | 102.17ms - 120.63ms | - | unsure π -12% - +10% -13.81ms - +10.93ms |
| branch | 241 kB | 104.61ms - 121.07ms | unsure π -10% - +12% -10.93ms - +13.81ms |
- |
slider permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 341 kB | 353.56ms - 364.60ms | - | unsure π -3% - +2% -9.76ms - +8.04ms |
| branch | 340 kB | 352.95ms - 366.93ms | unsure π -2% - +3% -8.04ms - +9.76ms |
- |
split-button permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 548 kB | 2172.35ms - 2183.01ms | - | unsure π -1% - +0% -12.66ms - +5.58ms |
| branch | 547 kB | 2173.82ms - 2188.62ms | unsure π -0% - +1% -5.58ms - +12.66ms |
- |
tooltip permalink
| Version | Bytes | Avg Time | vs remote | vs branch |
|---|---|---|---|---|
| npm latest | 248 kB | 81.02ms - 89.50ms | - | unsure π -8% - +8% -6.44ms - +6.60ms |
| branch | 247 kB | 80.22ms - 90.14ms | unsure π -8% - +8% -6.60ms - +6.44ms |
- |
@hunterloftis you helped us bring this to life to begin with, and there are a bunch of changes included to adopted the new CSS dependency, so I was hoping you could take a closer look and make sure this aligns with anything that might remain in your head regarding the implementation of this.
Sorry, I missed this GH ping @Westbrook.
Re: the similarity w/ action button, I think the guidance for users boils down to: Is this an interactive component?
- Yes: Action Button
- No: Badge
@lunarfusion where can I learn more about the 'fixed' variant being deprecated? That variant is used in several places in production & doesn't appear deprecated in the design spec.
The fixed version itself isnβt deprecated, the values for that attribute have been updated to be logical property based so inline-start rather than left, which is a choose of mine.
@Westbrook I must be missing something; the updated values look like colors? I'll take a closer look later.