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 to create system-overrides for S1/Express #4442

Open
wants to merge 140 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.

Next Steps

Ensure there are no breaks to the existing code before merging.

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.94 0.94
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 221.237 kB 210.431 kB 🏆 210.857 kB
Scripts 53.549 kB 48.169 kB 🏆 48.563 kB
Stylesheet 34.768 kB 30.373 kB 🏆 30.384 kB
Document 5.996 kB 5.285 kB 🏆 5.297 kB
Font 126.924 kB 126.604 kB 🏆 126.613 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 485 kB 51.86ms - 53.79ms - faster ✔
4% - 9%
1.93ms - 5.03ms
branch 500 kB 55.09ms - 57.52ms slower ❌
4% - 10%
1.93ms - 5.03ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 135.94ms - 138.66ms - faster ✔
6% - 9%
8.90ms - 13.41ms
branch 662 kB 146.66ms - 150.26ms slower ❌
6% - 10%
8.90ms - 13.41ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 62.67ms - 63.99ms - faster ✔
5% - 9%
3.58ms - 5.84ms
branch 619 kB 67.12ms - 68.95ms slower ❌
6% - 9%
3.58ms - 5.84ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 60.41ms - 61.54ms - faster ✔
7% - 9%
4.27ms - 6.05ms
branch 618 kB 65.45ms - 66.82ms slower ❌
7% - 10%
4.27ms - 6.05ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 790 kB 1873.19ms - 1876.44ms - unsure 🔍
-0% - +0%
-3.84ms - +0.84ms
branch 805 kB 1874.63ms - 1877.99ms unsure 🔍
-0% - +0%
-0.84ms - +3.84ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1857.25ms - 1859.84ms - unsure 🔍
-0% - -0%
-4.54ms - -0.81ms
branch 803 kB 1859.88ms - 1862.56ms unsure 🔍
+0% - +0%
+0.81ms - +4.54ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 38.11ms - 38.91ms - faster ✔
1% - 4%
0.45ms - 1.66ms
branch 517 kB 39.10ms - 40.01ms slower ❌
1% - 4%
0.45ms - 1.66ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 710 kB 35.58ms - 36.12ms - faster ✔
6% - 8%
2.33ms - 3.08ms
branch 725 kB 38.30ms - 38.81ms slower ❌
6% - 9%
2.33ms - 3.08ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 710 kB 395.50ms - 402.24ms - faster ✔
3% - 5%
11.02ms - 19.87ms
branch 725 kB 411.45ms - 417.18ms slower ❌
3% - 5%
11.02ms - 19.87ms
-
Firefox

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 485 kB 113.49ms - 118.31ms - faster ✔
1% - 7%
0.67ms - 8.53ms
branch 500 kB 117.39ms - 123.61ms slower ❌
1% - 7%
0.67ms - 8.53ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 274.76ms - 277.16ms - faster ✔
13% - 14%
39.93ms - 45.71ms
branch 662 kB 316.15ms - 321.41ms slower ❌
14% - 17%
39.93ms - 45.71ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 130.38ms - 133.02ms - unsure 🔍
-2% - +0%
-2.76ms - +0.24ms
branch 619 kB 132.26ms - 133.66ms unsure 🔍
-0% - +2%
-0.24ms - +2.76ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 152.92ms - 159.72ms - slower ❌
8% - 14%
10.79ms - 19.09ms
branch 618 kB 139.00ms - 143.76ms faster ✔
7% - 12%
10.79ms - 19.09ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 790 kB 1908.10ms - 1919.62ms - slower ❌
1% - 2%
18.42ms - 30.62ms
branch 805 kB 1887.33ms - 1891.35ms faster ✔
1% - 2%
18.42ms - 30.62ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1885.52ms - 1889.96ms - unsure 🔍
-0% - +0%
-3.60ms - +2.96ms
branch 803 kB 1885.64ms - 1890.48ms unsure 🔍
-0% - +0%
-2.96ms - +3.60ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 70.02ms - 72.10ms - faster ✔
2% - 6%
1.77ms - 4.79ms
branch 517 kB 73.25ms - 75.43ms slower ❌
2% - 7%
1.77ms - 4.79ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 710 kB 59.96ms - 63.00ms - faster ✔
1% - 7%
0.35ms - 4.21ms
branch 725 kB 62.56ms - 64.96ms slower ❌
0% - 7%
0.35ms - 4.21ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 710 kB 748.93ms - 759.63ms - slower ❌
2% - 5%
11.85ms - 36.31ms
branch 725 kB 719.20ms - 741.20ms faster ✔
2% - 5%
11.85ms - 36.31ms
-

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

@TarunAdobe TarunAdobe requested a review from a team June 10, 2024 06:02
@TarunAdobe TarunAdobe changed the title chore: updated tokens processing and now we're creating legacy-overrides for S1/Express chore: updated tokens processing to create legacy-overrides for S1/Express Jun 10, 2024
@TarunAdobe TarunAdobe changed the title chore: updated tokens processing to create legacy-overrides for S1/Express chore: updated tokens processing to create system-overrides for S1/Express Jun 10, 2024
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