spectrum-web-components icon indicating copy to clipboard operation
spectrum-web-components copied to clipboard

refactor(toast): use core tokens

Open pfulton opened this issue 3 years ago • 5 comments
trafficstars

This adds a beta version of the migrated-to-core-tokens Toast component.

Uses beta release, hold for full release.

Description

Related issue(s)

Motivation and context

How has this been tested?

  • [ ] Test case 1
    1. Go here
    2. Do this
  • [ ] Test case 2
    1. Go here
    2. 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.

pfulton avatar Aug 05 '22 18:08 pfulton

Tachometer results

Chrome

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 32.85ms - 33.11ms - faster ✔
1% - 2%
0.25ms - 0.67ms
branch 244 kB 33.28ms - 33.61ms slower ❌
1% - 2%
0.25ms - 0.67ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 214.92ms - 216.42ms - slower ❌
0% - 1%
0.36ms - 2.70ms
branch 608 kB 213.24ms - 215.04ms faster ✔
0% - 1%
0.36ms - 2.70ms
-

card permalink

Version Bytes Avg Time vs remote vs branch
npm latest 379 kB 86.16ms - 89.19ms - unsure 🔍
-1% - +2%
-1.05ms - +2.16ms
branch 391 kB 86.60ms - 87.64ms unsure 🔍
-2% - +1%
-2.16ms - +1.05ms
-

illustrated-message permalink

Version Bytes Avg Time vs remote vs branch
npm latest 282 kB 40.36ms - 40.68ms - faster ✔
0% - 2%
0.08ms - 0.66ms
branch 293 kB 40.64ms - 41.14ms slower ❌
0% - 2%
0.08ms - 0.66ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 298 kB 240.91ms - 243.69ms - unsure 🔍
-1% - +1%
-2.80ms - +1.81ms
branch 309 kB 240.96ms - 244.64ms unsure 🔍
-1% - +1%
-1.81ms - +2.80ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 73.59ms - 74.43ms - unsure 🔍
-1% - +1%
-0.49ms - +0.48ms
branch 347 kB 73.78ms - 74.26ms unsure 🔍
-1% - +1%
-0.48ms - +0.49ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 666.77ms - 685.32ms - unsure 🔍
-1% - +2%
-9.83ms - +16.42ms
branch 453 kB 663.46ms - 682.04ms unsure 🔍
-2% - +1%
-16.42ms - +9.83ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 30.51ms - 30.93ms - faster ✔
0% - 2%
0.13ms - 0.61ms
branch 241 kB 30.97ms - 31.21ms slower ❌
0% - 2%
0.13ms - 0.61ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 330 kB 146.51ms - 148.79ms - unsure 🔍
-2% - +0%
-2.58ms - +0.51ms
branch 341 kB 147.64ms - 149.73ms unsure 🔍
-0% - +2%
-0.51ms - +2.58ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 1949.18ms - 1952.26ms - unsure 🔍
-0% - +0%
-3.02ms - +1.42ms
branch 546 kB 1949.92ms - 1953.12ms unsure 🔍
-0% - +0%
-1.42ms - +3.02ms
-

toast permalink

Version Bytes Avg Time vs remote vs branch
npm latest 296 kB 49.70ms - 50.24ms - unsure 🔍
-1% - +0%
-0.68ms - +0.04ms
branch 308 kB 50.05ms - 50.53ms unsure 🔍
-0% - +1%
-0.04ms - +0.68ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 31.65ms - 32.15ms - unsure 🔍
-1% - +0%
-0.44ms - +0.14ms
branch 247 kB 31.90ms - 32.21ms unsure 🔍
-0% - +1%
-0.14ms - +0.44ms
-
Firefox

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 233 kB 86.13ms - 92.63ms - unsure 🔍
-8% - +2%
-7.27ms - +1.99ms
branch 244 kB 88.71ms - 95.33ms unsure 🔍
-2% - +8%
-1.99ms - +7.27ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 595 kB 365.70ms - 374.34ms - unsure 🔍
-3% - +1%
-9.54ms - +4.02ms
branch 608 kB 367.56ms - 378.00ms unsure 🔍
-1% - +3%
-4.02ms - +9.54ms
-

card permalink

Version Bytes Avg Time vs remote vs branch
npm latest 379 kB 147.88ms - 154.92ms - unsure 🔍
-3% - +3%
-4.92ms - +4.36ms
branch 391 kB 148.66ms - 154.70ms unsure 🔍
-3% - +3%
-4.36ms - +4.92ms
-

illustrated-message permalink

Version Bytes Avg Time vs remote vs branch
npm latest 282 kB 86.51ms - 102.57ms - slower ❌
2% - 26%
2.18ms - 21.18ms
branch 293 kB 77.78ms - 87.94ms faster ✔
3% - 22%
2.18ms - 21.18ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 298 kB 488.41ms - 496.59ms - faster ✔
1% - 3%
5.22ms - 17.62ms
branch 309 kB 499.26ms - 508.58ms slower ❌
1% - 4%
5.22ms - 17.62ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 336 kB 149.39ms - 157.13ms - unsure 🔍
-4% - +3%
-6.78ms - +4.62ms
branch 347 kB 150.15ms - 158.53ms unsure 🔍
-3% - +4%
-4.62ms - +6.78ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 442 kB 972.34ms - 979.38ms - unsure 🔍
-1% - +1%
-5.24ms - +5.68ms
branch 453 kB 971.47ms - 979.81ms unsure 🔍
-1% - +1%
-5.68ms - +5.24ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 230 kB 88.26ms - 106.02ms - slower ❌
13% - 42%
10.71ms - 31.21ms
branch 241 kB 71.06ms - 81.30ms faster ✔
13% - 30%
10.71ms - 31.21ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 330 kB 311.93ms - 318.87ms - unsure 🔍
-3% - +1%
-10.08ms - +1.84ms
branch 341 kB 314.68ms - 324.36ms unsure 🔍
-1% - +3%
-1.84ms - +10.08ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 534 kB 2047.57ms - 2061.87ms - faster ✔
0% - 1%
2.70ms - 23.22ms
branch 546 kB 2060.32ms - 2075.04ms slower ❌
0% - 1%
2.70ms - 23.22ms
-

toast permalink

Version Bytes Avg Time vs remote vs branch
npm latest 296 kB 113.35ms - 121.53ms - unsure 🔍
-4% - +6%
-4.35ms - +6.83ms
branch 308 kB 112.40ms - 120.00ms unsure 🔍
-6% - +4%
-6.83ms - +4.35ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 236 kB 75.86ms - 86.22ms - unsure 🔍
-5% - +13%
-3.49ms - +9.97ms
branch 247 kB 73.49ms - 82.11ms unsure 🔍
-12% - +4%
-9.97ms - +3.49ms
-

github-actions[bot] avatar Aug 05 '22 18:08 github-actions[bot]

Much bigger, namely in medium? The close button gets wonky there, too?

Westbrook avatar Aug 05 '22 19:08 Westbrook

Much bigger, namely in medium? The close button gets wonky there, too?

Yep, looking funky. We'll investigate.

pfulton avatar Aug 05 '22 21:08 pfulton

@yosevu Mind taking a look at this?

pfulton avatar Aug 25 '22 21:08 pfulton

It looks like there's a line-height being applied here, but I'm unsure of where it's coming from. It's overriding the component's defined line-height: var(--mod-toast-line-height, var(--spectrum-toast-line-height)); which would resolve to 1.3.

Screen_Shot_2022-09-23_at_1_49_30_PM

Any thoughts on where this might be coming from, @Westbrook?

pfulton avatar Sep 23 '22 17:09 pfulton

We apparently added it as part of the initial implementation of this: https://github.com/adobe/spectrum-web-components/blame/main/packages/toast/src/toast.css#L16

Please feel free to remove it if everything looks good to go with its removal!

Westbrook avatar Sep 26 '22 19:09 Westbrook

This is looking much better with the line-height adjustment. I've merged the CSS PR and graduated the beta. The newly released version is: - @spectrum-css/[email protected]

pfulton avatar Sep 28 '22 17:09 pfulton