Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: updated tokens processing and now we're creating legacy-overrides for S1/Express #4442

Open
wants to merge 137 commits into
base: main
Choose a base branch
from

Conversation

TarunAdobe
Copy link
Contributor

@TarunAdobe TarunAdobe commented May 10, 2024

Summary

This PR introduces changes to the Spectrum Web Components (SWC) project to facilitate the transition from Spectrum 1 (S1) to Spectrum 2 (S2) CSS. It includes the addition of a new index file (component/{bridge,spectrum,express}/index.css) in the @spectrum-css/tokens@14.1.0-alpha.3 package to assist in loading component-specific CSS. Additionally, it updates the SWC library to process the bridge file (referred to as legacy-overrides in SWC for now) and import it into the components.

Changes Made

1.@spectrum-css added a new bridge file in the @spectrum-css/tokens@14.1.0-alpha.3 package to aid in loading component-specific CSS from the tokens module directly.
2. Updated the generate-token script to include component's system levels in global-vars.css
3. Updated the process-spectrum script to create a legacy-override css file from the @spectrum-css/tokens.../bridge.css

Note: Investigated and identified VRT failures due to changes in global CSS tokens in the new sp-action-button from the CSS side. For example, --mod-actionbutton-content-color-default now points to gray-25 instead of gray-50 for the selected state, which might be a breaking change for S1.

Questions and Discussions

  1. Are there any global token changes directly in the component level CSS as we migrate to S2, considering they could impact S1 delivery?
  2. How should we handle gray remapping for S2 CSS with S1 tokens?
  3. How should we ensure Patrick's S2 CSS continues to work with S1 CSS?
  4. Do we need a new export that is just the CSS without the token values defined?

Next Steps

Ensure there are no breaks to the existing code before merging.
Implement a PR off SWC’s main that swaps out loading bridge tokens from the tokens package instead of the component packages, and ensure there are no regressions.

Motivation and context

This PR is important to bring in S1/Express core tokens with S2 overrides keeping in mind

  • can be reversed via a theme (back-portable to S1/express)
  • change without removing APIs
  • change without impacting layout

How has this been tested?

  • VRTs off the main branch should just work as it is !!

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.

jnjosh and others added 30 commits March 15, 2024 11:51
@TarunAdobe TarunAdobe closed this May 13, 2024
@TarunAdobe TarunAdobe reopened this May 13, 2024
Copy link

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:

Copy link

github-actions bot commented May 13, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.99 0.99 0.99
Accessibility 1 1 1
Best Practices 1 1 1
SEO 1 0.92 0.92
PWA 1 1 1
What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category Latest Main Branch
Total 222.503 kB 210.154 kB 🏆 210.247 kB
Scripts 54.819 kB 48.03 kB 48.021 kB 🏆
Stylesheet 34.939 kB 30.337 kB 🏆 30.397 kB
Document 5.893 kB 5.183 kB 🏆 5.189 kB
Font 126.852 kB 126.604 kB 🏆 126.64 kB

Request Count

Category Latest Main Branch
Total 45 45 45
Scripts 37 37 37
Stylesheet 5 5 5
Document 1 1 1
Font 2 2 2

Copy link

github-actions bot commented May 13, 2024

Tachometer results

Chrome

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 484 kB 49.92ms - 52.31ms - faster ✔
1% - 7%
0.24ms - 3.52ms
branch 500 kB 51.88ms - 54.11ms slower ❌
0% - 7%
0.24ms - 3.52ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 130.90ms - 133.57ms - faster ✔
5% - 8%
7.39ms - 11.91ms
branch 662 kB 140.06ms - 143.71ms slower ❌
6% - 9%
7.39ms - 11.91ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 60.15ms - 61.25ms - faster ✔
6% - 9%
4.09ms - 5.84ms
branch 619 kB 64.98ms - 66.35ms slower ❌
7% - 10%
4.09ms - 5.84ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 58.01ms - 58.95ms - faster ✔
7% - 9%
4.24ms - 6.03ms
branch 618 kB 62.85ms - 64.38ms slower ❌
7% - 10%
4.24ms - 6.03ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 1872.32ms - 1875.16ms - unsure 🔍
-0% - +0%
-1.38ms - +2.60ms
branch 805 kB 1871.73ms - 1874.52ms unsure 🔍
-0% - +0%
-2.60ms - +1.38ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1859.51ms - 1862.79ms - unsure 🔍
-0% - +0%
-1.14ms - +3.02ms
branch 803 kB 1858.93ms - 1861.49ms unsure 🔍
-0% - +0%
-3.02ms - +1.14ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 36.87ms - 37.48ms - faster ✔
2% - 4%
0.74ms - 1.68ms
branch 517 kB 38.03ms - 38.75ms slower ❌
2% - 5%
0.74ms - 1.68ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 34.57ms - 35.24ms - faster ✔
5% - 7%
1.98ms - 2.79ms
branch 724 kB 37.06ms - 37.52ms slower ❌
6% - 8%
1.98ms - 2.79ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 380.04ms - 385.39ms - faster ✔
4% - 6%
14.24ms - 22.15ms
branch 725 kB 398.00ms - 403.82ms slower ❌
4% - 6%
14.24ms - 22.15ms
-

illustrated-message permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 404 kB 13.39ms - 13.61ms - faster ✔
2% - 5%
0.31ms - 0.71ms
branch 421 kB 13.85ms - 14.17ms slower ❌
2% - 5%
0.31ms - 0.71ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 475 kB 203.81ms - 207.17ms - faster ✔
3% - 5%
5.34ms - 10.21ms
branch 491 kB 211.50ms - 215.03ms slower ❌
3% - 5%
5.34ms - 10.21ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 682 kB 416.75ms - 419.37ms - faster ✔
1% - 2%
3.42ms - 7.32ms
branch 706 kB 421.98ms - 424.88ms slower ❌
1% - 2%
3.42ms - 7.32ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 785 kB 21.42ms - 21.78ms - faster ✔
10% - 12%
2.28ms - 2.92ms
branch 792 kB 23.93ms - 24.47ms slower ❌
10% - 14%
2.28ms - 2.92ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 763 kB 343.10ms - 346.66ms - faster ✔
1% - 3%
5.26ms - 10.52ms
branch 778 kB 350.83ms - 354.70ms slower ❌
2% - 3%
5.26ms - 10.52ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 561 kB 40.11ms - 40.97ms - faster ✔
6% - 10%
2.74ms - 4.45ms
branch 576 kB 43.40ms - 44.87ms slower ❌
7% - 11%
2.74ms - 4.45ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 513 kB 516.76ms - 525.71ms - faster ✔
2% - 5%
12.40ms - 25.18ms
branch 528 kB 535.46ms - 544.59ms slower ❌
2% - 5%
12.40ms - 25.18ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 384 kB 11.38ms - 11.50ms - faster ✔
4% - 6%
0.50ms - 0.69ms
branch 400 kB 11.96ms - 12.11ms slower ❌
4% - 6%
0.50ms - 0.69ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 74.75ms - 76.46ms - faster ✔
2% - 6%
1.80ms - 4.47ms
branch 495 kB 77.72ms - 79.77ms slower ❌
2% - 6%
1.80ms - 4.47ms
-

split-button permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 723 kB 1855.91ms - 1859.49ms - unsure 🔍
-0% - +0%
-2.36ms - +2.38ms
branch 738 kB 1856.15ms - 1859.24ms unsure 🔍
-0% - +0%
-2.38ms - +2.36ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 548 kB 32.96ms - 33.94ms - faster ✔
3% - 6%
0.91ms - 2.11ms
branch 566 kB 34.61ms - 35.31ms slower ❌
3% - 6%
0.91ms - 2.11ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 533 kB 22.74ms - 23.29ms - faster ✔
5% - 8%
1.30ms - 2.11ms
branch 562 kB 24.43ms - 25.02ms slower ❌
6% - 9%
1.30ms - 2.11ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 50.43ms - 51.32ms - faster ✔
5% - 7%
2.56ms - 3.90ms
branch 671 kB 53.60ms - 54.61ms slower ❌
5% - 8%
2.56ms - 3.90ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 632 kB 40.96ms - 41.77ms - faster ✔
7% - 10%
3.12ms - 4.32ms
branch 647 kB 44.64ms - 45.53ms slower ❌
7% - 11%
3.12ms - 4.32ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 528 kB 54.25ms - 55.03ms - faster ✔
4% - 6%
2.34ms - 3.65ms
branch 544 kB 57.10ms - 58.16ms slower ❌
4% - 7%
2.34ms - 3.65ms
-
Firefox

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 484 kB 109.46ms - 114.94ms - faster ✔
1% - 9%
1.08ms - 10.20ms
branch 500 kB 114.20ms - 121.48ms slower ❌
1% - 9%
1.08ms - 10.20ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 270.50ms - 273.30ms - faster ✔
13% - 14%
39.92ms - 45.04ms
branch 662 kB 312.23ms - 316.53ms slower ❌
15% - 17%
39.92ms - 45.04ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 126.38ms - 128.46ms - faster ✔
2% - 4%
2.85ms - 5.35ms
branch 619 kB 130.82ms - 132.22ms slower ❌
2% - 4%
2.85ms - 5.35ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 153.23ms - 157.73ms - slower ❌
10% - 15%
13.45ms - 20.23ms
branch 618 kB 136.10ms - 141.18ms faster ✔
9% - 13%
13.45ms - 20.23ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 1916.62ms - 1924.18ms - slower ❌
2% - 2%
29.40ms - 38.04ms
branch 805 kB 1884.58ms - 1888.78ms faster ✔
2% - 2%
29.40ms - 38.04ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1883.95ms - 1889.69ms - unsure 🔍
+0% - +0%
+1.20ms - +8.00ms
branch 803 kB 1880.38ms - 1884.06ms unsure 🔍
-0% - -0%
-8.00ms - -1.20ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 76.55ms - 82.21ms - unsure 🔍
-8% - +2%
-6.26ms - +1.34ms
branch 517 kB 79.30ms - 84.38ms unsure 🔍
-2% - +8%
-1.34ms - +6.26ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 62.52ms - 68.80ms - unsure 🔍
-0% - +11%
-0.05ms - +6.89ms
branch 724 kB 60.76ms - 63.72ms unsure 🔍
-10% - -0%
-6.89ms - +0.05ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 714.05ms - 732.23ms - slower ❌
2% - 5%
15.77ms - 36.27ms
branch 725 kB 692.39ms - 701.85ms faster ✔
2% - 5%
15.77ms - 36.27ms
-

illustrated-message permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 404 kB 25.48ms - 27.36ms - unsure 🔍
-6% - +3%
-1.72ms - +0.72ms
branch 421 kB 26.15ms - 27.69ms unsure 🔍
-3% - +7%
-0.72ms - +1.72ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 475 kB 423.75ms - 436.57ms - faster ✔
2% - 6%
7.83ms - 26.13ms
branch 491 kB 440.61ms - 453.67ms slower ❌
2% - 6%
7.83ms - 26.13ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 770 kB 622.98ms - 631.70ms - slower ❌
2% - 5%
11.41ms - 27.55ms
branch 785 kB 601.07ms - 614.65ms faster ✔
2% - 4%
11.41ms - 27.55ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 773 kB 45.35ms - 46.13ms - faster ✔
5% - 8%
2.49ms - 4.11ms
branch 788 kB 48.33ms - 49.75ms slower ❌
5% - 9%
2.49ms - 4.11ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 763 kB 645.15ms - 651.05ms - slower ❌
4% - 5%
24.34ms - 32.94ms
branch 778 kB 616.32ms - 622.60ms faster ✔
4% - 5%
24.34ms - 32.94ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 561 kB 99.09ms - 107.91ms - slower ❌
8% - 17%
6.94ms - 15.82ms
branch 576 kB 91.70ms - 92.54ms faster ✔
7% - 15%
6.94ms - 15.82ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 513 kB 985.99ms - 1011.69ms - faster ✔
3% - 6%
31.39ms - 58.37ms
branch 528 kB 1039.63ms - 1047.81ms slower ❌
3% - 6%
31.39ms - 58.37ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 384 kB 29.26ms - 34.06ms - unsure 🔍
-13% - +7%
-4.30ms - +2.22ms
branch 400 kB 30.50ms - 34.90ms unsure 🔍
-7% - +14%
-2.22ms - +4.30ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 162.36ms - 169.48ms - faster ✔
0% - 7%
0.43ms - 11.89ms
branch 495 kB 167.59ms - 176.57ms slower ❌
0% - 7%
0.43ms - 11.89ms
-

split-button permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 723 kB 1947.42ms - 2036.82ms - unsure 🔍
-5% - +2%
-93.73ms - +41.65ms
branch 738 kB 1967.33ms - 2068.99ms unsure 🔍
-2% - +5%
-41.65ms - +93.73ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 80.12ms - 84.44ms - slower ❌
11% - 18%
7.99ms - 12.73ms
branch 671 kB 70.94ms - 72.90ms faster ✔
10% - 15%
7.99ms - 12.73ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 533 kB 45.90ms - 46.74ms - faster ✔
29% - 33%
18.57ms - 23.11ms
branch 549 kB 64.93ms - 69.39ms slower ❌
40% - 50%
18.57ms - 23.11ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 108.20ms - 113.60ms - faster ✔
7% - 14%
8.45ms - 17.35ms
branch 671 kB 120.26ms - 127.34ms slower ❌
7% - 16%
8.45ms - 17.35ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 632 kB 91.19ms - 96.33ms - faster ✔
5% - 11%
4.68ms - 11.56ms
branch 647 kB 99.58ms - 104.18ms slower ❌
5% - 13%
4.68ms - 11.56ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 528 kB 98.52ms - 104.32ms - unsure 🔍
-8% - +0%
-8.02ms - +0.26ms
branch 544 kB 102.35ms - 108.25ms unsure 🔍
-0% - +8%
-0.26ms - +8.02ms
-

@TarunAdobe TarunAdobe changed the title [DO NOT MERGE]!: Testing spectrum-css's S2 bridge chore: updated tokens processing and now we're creating legacy-overrides for S1/Express May 17, 2024
@TarunAdobe TarunAdobe marked this pull request as ready for review May 17, 2024 04:13
@TarunAdobe TarunAdobe self-assigned this May 17, 2024
// check if spectrumPath exists
if (fs.existsSync(spectrumPath)) {
let spectrum = fs.readFileSync(spectrumPath, 'utf8');
spectrum = removeImporantComments(targetHost(spectrum));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
spectrum = removeImporantComments(targetHost(spectrum));
spectrum = removeImportantComments(targetHost(spectrum));

Small spelling note ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh question - I'm assuming this function removes the copyright which you're also adding to files - why not just keep the copyright and strip duplicates after merge? The postcss-discard-comments plugin offers a removeAllButFirst option which is a nice way to preserve the first copyright in the file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants