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

Pre-v3 prep #2019

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Pre-v3 prep #2019

merged 1 commit into from
Aug 16, 2024

Conversation

anselmbradford
Copy link
Member

These are adjustments based on what's needed in cfpb/consumerfinance.gov#8514
Next steps would be merging this, releasing v3.x, and then updating 8514

Explanation of changes is below:

  • I removed the underscore prefix to some scss filenames. This is a Sass convention on files that are included in other files. However, underscore or no underscore works, and if we have index.js and _index.scss they won't appear together in the file order, which seems wrong. Best to not bake in magic characters in the filenames.

  • CSS custom variables (custom properties) are a runtime variable that is processed by the browser. This means when the scss files are built, it doesn't know which CSS variables are used or not, so it includes them all wherever the scss from the DS are used. This leads to a lot of duplicates. Instead, they should be included once in the main CSS so that they are available globally, but defined in only one place. To accomplish this, I wrapped them in a Sass mixin, which means they will only be included where the Sass mixin is included.

  • Some variables are in components and some are in the global vars in the /abstracts/ directory. I moved some more vars to the global space that are referenced elsewhere in the cfgov codebase. Eventually, placing them in one space or the other should be sorted out to be more consistent. This can be re-examined in a web component future.

Changes

  • Remove underscore named files.
  • Move custom props to mixin.
  • Move some component vars to the global scope.
  • Remove default export from breakpoints.

Testing

  1. PR checks should pass.

@anselmbradford anselmbradford added the lerna-changelog/enhancement lerna label. DO NOT MODIFY label Aug 16, 2024
Copy link

netlify bot commented Aug 16, 2024

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

Name Link
🔨 Latest commit 20b9076
🔍 Latest deploy log https://app.netlify.com/sites/cfpb-design-system/deploys/66bfc484e69fcd0008bc72b0
😎 Deploy Preview https://deploy-preview-2019--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 merged commit 301eae0 into main Aug 16, 2024
8 checks passed
@anselmbradford anselmbradford deleted the ans_moar_cleanup branch August 16, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lerna-changelog/enhancement lerna label. DO NOT MODIFY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant