Skip to content

CSS architecture changes#2646

Draft
itsmedavep wants to merge 3 commits into
mainfrom
dave_css_architecture_changes
Draft

CSS architecture changes#2646
itsmedavep wants to merge 3 commits into
mainfrom
dave_css_architecture_changes

Conversation

@itsmedavep
Copy link
Copy Markdown
Collaborator

This branch changes the CSS architects in four ways:

  1. It stops sass API imports from emitting duplicate global CSS
  2. It stops web component CSS from leaking into the global style sheet package
  3. It makes token scope explict for legacy CSS vs shadow-DOM CSS
  4. It starts separating global CSS from "web component global" CSS in the source organization

We had and issue where we were duplicating :root and :host styles in the package.

The result is that the packaged stylesheet drops from about 96.9 kB, and 2 :root blocks , and 21 :host selectors down to roughly 83.9 kB , a single :root block, and 0 `:host' selectors in the package CSS.

Additions

Removals

Changes

  1. The biggest change is the abstracts split. On main elements/abstracts/index.scss is both:
  • Sass API for mixins/vars
  • A CSS emitting entry the forwards tokens files

That is the core duplication issue. Here I have introduced elements/abstracts/api.scss as the non-emitting Sass API, and updated a large set of legacy component and element styles to import that instead of the abstracts entry that emitted CSS. The same idea was applied to sizing with sizing-core.scss so Sass values can be reused without repeaditly re-emitting root CSS.

  1. The second big change is removing web component forwards from the global package stylesheet. On main src/index.scss forwards both legacy component CSS and web component element CSS. That was why main leaked :host rules into the CSS package.

In this branch src/index.scss is reduced to the composition of:
elements/index.scss
components/index.scss
utilities/index.scss

That means that the actual web component styles.component.scss files are no longer being forwarded into the package.

  1. The next change was to token scoping. Style dictionary config now generates both :root and :host token outputs from the JSON tokens. That change added generated files like:

src/elements/abstracts/custom-props.host.css
'src/elements/abstracts/custom-prop.css`

Legacy components still use the :root tokens files. Web components use the :host files where needed.

  1. The fourth change was around the global css source organization. elements/base/base.css no longer imports the skeleton/global custom-element files. Those moved into elements/cfpb-utilities/global.scss while the global tokens are grouped in elements/abstracts/tokens.scss.

What that provided was a clean conceptual split:
components' = legacy class based CSS elements= web component foundation CSSabstracts` = tokens/foundations

  1. I also added a few mixin only files:

utilities/mixins.scss
elements/cfpb-button/mixins.scss
elements/cfpb-icon-text/mixin.scss

That was needed because some files were importing CSS emitting only to use a mixin. That is the dependancy that creates duplication.

Bottom line is that change gets us smaller CSS and makes sure that future component additions are less likely to reintroduce the the :root duplication or the :host inclusion in the package. A new WC won't have to rely on the global package accidentally carrying its variables or styles. A new legacy component doesn't have to worry about pulling in a Sass helper that will duplicate tokens. (probably not terribly important since we are not going to be introducing legacy components).

It should help with architectural clarity:

  • package global CSS is now truly global CSS
  • web component globals are seperate from legacy base styles
  • token generation from JSON now supports both legacy and web component consumers from the same source of truth

This approach replaces accidental coupling of things with being explicit:

  • non-emitting Sass API vs emitting CSS
  • legacy/global CSS vs web component CSS
  • :root tokens vs :host tokens
  • legacy base vs custom element

Testing

  1. Verified CSS package changes dist/index.css by yarn build and comparing size to main and then searching for :root duplication and :host inclusion.
  2. Visually validated that the DS website did not regress nor did any of the legacy or web components.

Screenshots

Before After

Notes and todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the CFPB development guidelines
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Browsers

Check the current browser support cutoff list for browsers that are advisable
to prioritize for testing.

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant