-
Notifications
You must be signed in to change notification settings - Fork 225
ci, docs: Remove Spectrum CSS dependencies and processing #5472
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
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving some comments after reviewing documentation in this PR.
I didn't check #5419 and it's very possible that some of what's mentioned below is covered there:
After checking, I didn't see any remaining spectrum-config.js
but I did see a few lingering references to spectrum-config.js
, for instance we have spectrum-config.js.hbs
still, and spectrum-config.js
is still mentioned in the .gitignore, package.json, and it looks like in a few tasks (build-component-inventory.js
, process-spectrum-js
), those might be opportunities for more cleanup.
There were also a few references to spectrum-css
that seemed stale but might also be in the other PR:
- Could we clean up any of the files listed in the
"process-spectrum"
build step? - There are also a few scripts in package.json starting with
update:spectrum-css
, wondering if there's any cleanup opportunity there @spectrum-css
also mentioned inrenovate.json
, not sure if we're ready to take that out now or want to do that later.
I'm coming to this without a complete understanding of how things work in SWC, so it's possible that some of these items are correct and/or still necessary, but sharing my observations either way!
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
* chore: remove spectrum-config.js files from various packages * chore; remove spectrum-css dependencies * chore: remove unused @spectrum-css dependencies from yarn.lock * Update packages/icons-ui/package.json * Update packages/icons-ui/package.json --------- Co-authored-by: Casey Eickhoff <[email protected]>
* chore: add styling guide * chore: remove spectrum-config and update other docs * fix: update yarn new-package to not depend on spectrum-css * chore: update styling guide * chore: remove formating from plopfile
Co-authored-by: rise-erpelding <[email protected]>
Co-authored-by: rise-erpelding <[email protected]>
* chore: move tasks into scripts [swc-854] * chore: update custom element manifest scripts to simpify
* chore: remove build-confirm command and fix cem-tools * chore: adds build confirm command back --------- Co-authored-by: Casey Eickhoff <[email protected]>
Main README does not match the anatomy of Generating Components guide. |
In
In
In
|
@castastrophe can i ask for you to take a look at the script Edit: I added a @todo to that file. I presently doesn't do anything or affect anything as its been disabled for some time but feels out of scope for this initiative since its existed in this state prior to cutoff. |
@nikkimk Icons will still rely on Spectrum CSS so those observations are expected, I've updated the description. |
@castastrophe can i also get your help on the CSS hot reload issue @nikkimk called out tomorrow? I can look deeper in the morning but thought you might know whats up |
The storybook hot refresh issue with child stylesheets is a known issue that exists today in both SWC and SP-CSS. I can file a spike ticket to investigate how we might get this working but its an issue with storybook and web components playing nicely. For the sake of this PR we will accept that behavior since its currently present. I will update the tests and description with this context too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lofty goal totally met! (LGTM)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request is not just acceptable...
pause
It's a shining beacon of engineering brilliance, deserving of song, dance and immediate merge. (LGTM)
When searching for |
Co-authored-by: Josh Johnson <[email protected]>
Co-authored-by: Josh Johnson <[email protected]>
Co-authored-by: Josh Johnson <[email protected]>
Style Management in Spectrum Web Components
This PR documents the guidance for maintaining styles in SWC and removes external style dependencies.
Stylesheet Organization
spectrum-[component].css
: Base stylesheet from spectrum-css[component]-overrides.css
: Overrides stylesheet from spectrum-css[component].css
: SWC-specific override stylesheet (loaded last)Build Process Changes
spectrum-*.css
and*-overrides.css
files are preserved during buildsyarn lint:version
withyarn constraints
for package version alignmentRemoved Items
Motivation and context
This PR establishes a self-contained styling system for Spectrum Web Components by eliminating external style dependencies and providing comprehensive documentation. Previously, our reliance on @spectrum-css created maintenance overhead, version compatibility issues, and complicated the developer experience. By internalizing all style assets and clarifying the purpose of each stylesheet type, we're simplifying the development workflow and ensuring long-term maintainability.
This change is required to provide clear and thorough styling guidance for developers working with SWC components. It addresses the confusion around the usage of multiple stylesheets and ensures that anyone can understand how to update themes and style new components effectively.
The migration of task scripts and introduction of the yarn constraints command further standardize our development processes, making the codebase more accessible to contributors and reducing the likelihood of inconsistencies across components.
Related issue(s)
Author's checklist
Reviewer's checklist
patch
,minor
, ormajor
featuresManual Testing Steps
Documentation Verification
Package Generation Testing
yarn new-package
works correctly:yarn && yarn build
yarn new-package
Dependency Removal Verification
Verify all instances are removed by checking these search queries:
NOTE: Spectrum-CSS will still remain a dependency for all icon-related packages. Please disregard that in the final review.
Build and Rendering Testing
yarn build
and verify no regressionsyarn start
and check for rendering issues in StorybookTheme and Style Testing
Style Preservation Verification
spectrum-[component].css
and[component]-overrides.css
files exist and are not modifiedConstraints Command Testing
yarn constraints
to verify it reports no issuesyarn constraints
to report the mismatchyarn constraints --fix
to correct the mismatch by updating to the most recent versionLive Update Testing
Note: The storybook hot refresh issue with child stylesheets is a known issue that exists today in both SWC and SP-CSS. For the sake of this PR we will accept that behavior since it's currently present. To test that the changes carry through, add a white space to the primary stylesheet and save to get Storybook to refresh.
Command Testing