Skip to content

Conversation

@najikahalsema
Copy link
Member

Description

Some of the files within the Spectrum CSS core tokens packages weren't being imported to be used in the theme tools, even though we are importing them in the styles tools. Added imports for these files so that we can consume these custom variables, as well.

Related issue(s)

Motivation and context

Missing import

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.

@najikahalsema najikahalsema requested a review from Westbrook June 26, 2023 21:09
@najikahalsema najikahalsema changed the title chore{theme): include custom var css files in theme imports chore(theme): include custom var css files in theme imports Jun 26, 2023
Copy link
Contributor

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

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

Great start. Don't forget to do the same for the Express files.

@najikahalsema najikahalsema force-pushed the halsema/include-custom-vars-imports branch from a49b957 to 5f26022 Compare June 26, 2023 21:25
@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

Tachometer results

Chrome

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 365 kB 38.02ms - 38.49ms - faster ✔
2% - 4%
0.78ms - 1.72ms
branch 364 kB 39.10ms - 39.92ms slower ❌
2% - 5%
0.78ms - 1.72ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 732 kB 205.87ms - 210.96ms - unsure 🔍
-1% - +2%
-2.28ms - +5.10ms
branch 731 kB 204.33ms - 209.68ms unsure 🔍
-2% - +1%
-5.10ms - +2.28ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 416 kB 273.05ms - 280.15ms - unsure 🔍
-0% - +3%
-0.96ms - +8.11ms
branch 415 kB 270.21ms - 275.84ms unsure 🔍
-3% - +0%
-8.11ms - +0.96ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 452 kB 81.00ms - 83.15ms - unsure 🔍
-0% - +3%
-0.26ms - +2.27ms
branch 452 kB 80.40ms - 81.75ms unsure 🔍
-3% - +0%
-2.27ms - +0.26ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 587 kB 677.38ms - 688.19ms - unsure 🔍
-1% - +1%
-7.68ms - +8.33ms
branch 586 kB 676.56ms - 688.36ms unsure 🔍
-1% - +1%
-8.33ms - +7.68ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 361 kB 37.17ms - 38.58ms - unsure 🔍
-4% - +1%
-1.45ms - +0.46ms
branch 361 kB 37.73ms - 39.02ms unsure 🔍
-1% - +4%
-0.46ms - +1.45ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 448 kB 166.35ms - 169.25ms - unsure 🔍
-3% - +0%
-4.76ms - +0.06ms
branch 447 kB 168.22ms - 172.08ms unsure 🔍
-0% - +3%
-0.06ms - +4.76ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 665 kB 2037.83ms - 2041.49ms - unsure 🔍
-0% - -0%
-6.19ms - -0.21ms
branch 664 kB 2040.50ms - 2045.22ms unsure 🔍
+0% - +0%
+0.21ms - +6.19ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 356 kB 37.58ms - 38.07ms - unsure 🔍
-1% - +1%
-0.34ms - +0.29ms
branch 356 kB 37.64ms - 38.06ms unsure 🔍
-1% - +1%
-0.29ms - +0.34ms
-
Firefox

action-bar permalink

Version Bytes Avg Time vs remote vs branch
npm latest 365 kB 103.70ms - 120.74ms - faster ✔
9% - 26%
10.73ms - 36.11ms
branch 364 kB 126.24ms - 145.04ms slower ❌
8% - 33%
10.73ms - 36.11ms
-

action-menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 732 kB 347.05ms - 364.67ms - unsure 🔍
-6% - +2%
-20.42ms - +8.94ms
branch 731 kB 349.86ms - 373.34ms unsure 🔍
-3% - +6%
-8.94ms - +20.42ms
-

menu permalink

Version Bytes Avg Time vs remote vs branch
npm latest 416 kB 476.09ms - 491.83ms - unsure 🔍
-6% - +0%
-31.36ms - +1.20ms
branch 415 kB 484.78ms - 513.30ms unsure 🔍
-0% - +7%
-1.20ms - +31.36ms
-

overlay permalink

Version Bytes Avg Time vs remote vs branch
npm latest 452 kB 160.91ms - 177.65ms - unsure 🔍
-7% - +7%
-11.48ms - +12.00ms
branch 452 kB 160.79ms - 177.25ms unsure 🔍
-7% - +7%
-12.00ms - +11.48ms
-

picker permalink

Version Bytes Avg Time vs remote vs branch
npm latest 587 kB 912.88ms - 940.16ms - unsure 🔍
-2% - +2%
-23.29ms - +14.73ms
branch 586 kB 917.56ms - 944.04ms unsure 🔍
-2% - +3%
-14.73ms - +23.29ms
-

popover permalink

Version Bytes Avg Time vs remote vs branch
npm latest 361 kB 75.22ms - 89.26ms - faster ✔
27% - 42%
31.99ms - 55.33ms
branch 361 kB 116.58ms - 135.22ms slower ❌
36% - 70%
31.99ms - 55.33ms
-

slider permalink

Version Bytes Avg Time vs remote vs branch
npm latest 448 kB 323.35ms - 348.29ms - unsure 🔍
-3% - +6%
-8.83ms - +21.15ms
branch 447 kB 321.33ms - 337.99ms unsure 🔍
-6% - +3%
-21.15ms - +8.83ms
-

split-button permalink

Version Bytes Avg Time vs remote vs branch
npm latest 665 kB 2129.01ms - 2139.67ms - faster ✔
0% - 1%
0.01ms - 15.87ms
branch 664 kB 2136.40ms - 2148.16ms unsure 🔍
-0% - +1%
+0.01ms - +15.87ms
-

tooltip permalink

Version Bytes Avg Time vs remote vs branch
npm latest 356 kB 82.70ms - 96.74ms - unsure 🔍
-13% - +8%
-12.12ms - +7.92ms
branch 356 kB 84.67ms - 98.97ms unsure 🔍
-9% - +14%
-7.92ms - +12.12ms
-

@najikahalsema
Copy link
Member Author

vrts are failing as expected on Dragged dropzone story, as those vars weren't getting into the component before now.

@Westbrook
Copy link
Contributor

Looks good here! Cycle the VRT hash and this should be good to go.

@najikahalsema najikahalsema force-pushed the halsema/include-custom-vars-imports branch from b10c30a to e9e9419 Compare June 27, 2023 17:34
@najikahalsema najikahalsema force-pushed the halsema/include-custom-vars-imports branch from 25bc303 to 31192df Compare June 27, 2023 18:14
@najikahalsema najikahalsema requested a review from Westbrook June 27, 2023 18:42
Copy link
Contributor

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

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

LGTM!

@Westbrook Westbrook merged commit 84e56a5 into main Jun 27, 2023
@Westbrook Westbrook deleted the halsema/include-custom-vars-imports branch June 27, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants