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

Convert Less to Sass #1993

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Convert Less to Sass #1993

merged 2 commits into from
Jul 18, 2024

Conversation

anselmbradford
Copy link
Member

@anselmbradford anselmbradford commented Jun 11, 2024

Changes

  • Convert docs site to scss from less.

  • Add scss to packages.

  • Change grid__push to u-grid-push.

  • Change grid__nested-col-group to u-grid-nested-col-group.

  • Change grid__column to u-grid-column.

  • Change u-link--colors to u-link-colors.

  • Change u-link--colors-base to u-link-colors-base.

  • Change u-link--no-border to u-link-no-border.

  • Change u-link--hover-border to u-link-hover-border.

  • Change u-link--border to u-link-border.

  • Change @grid_gutter-width to $grid-gutter-width

  • Change @block--bg to $block-bg.

Testing

  1. yarn install && yarn build && yarn build-packages
  2. Deploy preview https://deploy-preview-1993--cfpb-design-system.netlify.app/design-system/ should look okay.

Notes

  • Sass deprecates division using /, and instead uses the namespaced math.div(…,…).

Todo

  • The import works differently in Sass than it does in Less. In our Less configuration, the imports will weed out duplicates. In the Sass config, it will not weed out duplicates, so each package that imports other packages will get the other package's contents, which means the built dependencies are duplicated across the individual packages. Arguably this is correct. Each package should be able to be used independently of the others. However, how we currently use the packages is to install each one individually. Instead of working out removing this duplication, what I will be working on is having one published package, cfpb-design-system, from which each component can be extracted. In the interim, what we should do in cfgov is to import cfpb-design-system.scss instead of the individual packages. Some of the other imports will need to be sorted out on a case-by-case basis, since import (reference) doesn't have an equivalent in sass (see https://stackoverflow.com/questions/22851662/what-is-the-equivalent-of-lesss-import-reference-style-in-sass).

Copy link

netlify bot commented Jun 11, 2024

Thanks for the improvements! Browse a preview of your changes using the link below.

Name Link
🔨 Latest commit c95501c
🔍 Latest deploy log https://app.netlify.com/sites/cfpb-design-system/deploys/66915755108e4800082e72b7
😎 Deploy Preview https://deploy-preview-1993--cfpb-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@anselmbradford anselmbradford force-pushed the ans_sass branch 4 times, most recently from fbe1149 to 2841f2d Compare June 14, 2024 22:01
@anselmbradford anselmbradford force-pushed the ans_sass branch 6 times, most recently from 298d90b to 07673f8 Compare June 28, 2024 18:59
@anselmbradford anselmbradford marked this pull request as ready for review June 28, 2024 19:02
@anselmbradford anselmbradford requested a review from a team June 28, 2024 19:02
@anselmbradford anselmbradford added the lerna-changelog/breaking lerna label. DO NOT MODIFY label Jul 2, 2024
Copy link
Member

@contolini contolini left a comment

Choose a reason for hiding this comment

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

Using both manual and automated checks, I haven't been able to find any visual differences between the staging site and prod so everything seems a-okay.

@anselmbradford anselmbradford merged commit 8604e51 into main Jul 18, 2024
8 checks passed
@anselmbradford anselmbradford deleted the ans_sass branch July 18, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lerna-changelog/breaking lerna label. DO NOT MODIFY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants