-
Notifications
You must be signed in to change notification settings - Fork 12
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
CSS Library: Migrate VA styles from Formation #1124
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.
I've added the Bitter font files (from Formation) here which will copy over into dist/fonts
.
I have also loaded USWDS v3 @font-face
font imports which loads the font face for and references the font files in the USWDS package for these:
- Roboto Mono Web
- Source Sans Pro Web
- Merriweather Web
@use "uswds-fonts"; |
@use 'uswds-core' with ( | |
$theme-show-notifications: false, | |
// TODO: verify USWDS font path | |
$theme-font-path: '../node_modules/@uswds/uswds/dist/fonts', | |
); |
The problem though is that they have added "Web" to all of these font names so at some point we will need to update wherever "Source Sans Pro" is referenced to "Source Sans Pro Web". The same with "Merriweather" to "Merriweather Web", etc.
@@ -11,15 +11,19 @@ | |||
"build:tokens": "style-dictionary build", | |||
"build:stylesheets": "sass --load-path=../../node_modules/@uswds/uswds/packages/ src/stylesheets:dist/", | |||
"build:minify": "yarn build:stylesheets --style compressed --no-source-map", | |||
"build": "yarn run copy && yarn build:tokens && yarn build:minify", | |||
"copy": "node ./copy-uswds-color-tokens.js" | |||
"build:minify-core": "sass --style compressed --no-source-map dist/core.css:dist/core.min.css", |
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.
The build:minify-core
script isn't being used right now but I'm going to leave it here as a working example of how we could generate minified stylesheet(s) if necessary.
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.
I have added just the images from Formation that were being referenced in the stylesheets that were moved over so they would still work. They will be copied over to dist/img
.
But I expect that we wouldn't actually need any of these images in the CSS Library eventually. All necessary images would instead be coming from component-library or USWDS directly.
@@ -0,0 +1,434 @@ | |||
//============ PORTED MIXINS FROM FORMATION ==============// |
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.
There is zero organization for the mixins in this file because I don't expect any of these to survive the Formation culling. I just moved over whatever mixins were necessary to keep the ported stylesheets working.
@@ -0,0 +1,159 @@ | |||
@use 'sass:math'; |
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.
I had to make some modifications to this file in order for it to work with dart-sass
but I expect that this will also eventually go away as we remove stylesheets.
@@ -0,0 +1,189 @@ | |||
// This file is a place where you put all the code you're not proud of, |
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.
- Source Sans Pro Web | ||
- Merriweather Web | ||
*/ | ||
@use "uswds-fonts"; |
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 will add @font-face
imports for Roboto Mono Web, Source Sans Pro Web, and Merriweather Web from the USWDS v3 package. They all include "Web" in the name though which is different from v1.
@@ -0,0 +1,374 @@ | |||
@import '../override-function'; | |||
|
|||
@import 'uswds-core/src/styles/mixins/typography/usa-content-styles'; |
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 stylesheet is the same as elements.scss
.
This file is the "16px root font" version and elements.scss
is the "10px root font" version. The elements.scss file is being used in the component-library currently.
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.
Is this file actually using 16px as its root font size? I see font-size: $em-base
below on line 14, and saw that variable being set in /formation-overrides/_variables.scss as 10px.
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.
lol ignore me, I just remembered we're setting font-size on the body directly on line 23.
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.
Moved this fonts stylesheet to /stylesheets/base/_b-font-faces.scss
to mirror the Formation name and structure.
$h5-font-size: scale-rem(1.5rem); | ||
|
||
$color-black-light: #212121; | ||
$color-base: $color-black-light; |
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.
All of the tokens in this stylesheet are "imposter tokens". They are only here so that the ported stylesheets continue to work. These should all go away as we remove things though.
); | ||
|
||
$font-path: "./fonts"; // TODO: Verify asset path | ||
$image-path: "./img"; // TODO: Verify asset path |
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.
These asset paths point to where the assets will be in dist/
but when we start connecting stylesheets, this will need to be verified that it's correct.
See: https://github.com/uswds/uswds/blob/98566ec5dc12807a2b0bc36699a033fe3a711d44/packages/uswds-global/_index.scss | ||
*/ | ||
|
||
@use "uswds-elements/lib/normalize"; |
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.
I think we can just start using the USWDS v3 Normalize.css import instead of trying to keep using the one from USWDS v1 (which Formation was importing).
But USWDS v1 uses normalize v3.0.3 and USWDS v3 is using v8.0.1
I'm not sure if there will be any significant changes since Normalize just makes browsers render HTML elements more consistently but just something I wanted to call out.
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.
I'm in favor of moving to V3's normalize.
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.
I agree with Andrew. Where impact is minimal, we should try and get onto V3 stuff as we go.
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.
I think this looks good for getting us to a spot where we can cut over from Formation to CSS-Library and begin swapping imports. Exciting!
We'll have to do some digging around the uswds repo as a group, but my hope is that we'll be able to replace a lot of this stuff with direct imports from uswds-core
@import './override-function.scss'; | ||
|
||
body { | ||
// used a lot in vets-website |
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.
😭
@@ -0,0 +1,374 @@ | |||
@import '../override-function'; | |||
|
|||
@import 'uswds-core/src/styles/mixins/typography/usa-content-styles'; |
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.
Is this file actually using 16px as its root font size? I see font-size: $em-base
below on line 14, and saw that variable being set in /formation-overrides/_variables.scss as 10px.
@@ -0,0 +1,374 @@ | |||
@import '../override-function'; | |||
|
|||
@import 'uswds-core/src/styles/mixins/typography/usa-content-styles'; |
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.
lol ignore me, I just remembered we're setting font-size on the body directly on line 23.
This reverts commit 33ebc51.
Chromatic
https://2537-formation-styles-to-css-library--65a6e2ed2314f7b8f98609d8.chromatic.com
Description
What is this PR doing?
node-sass
functionality was being used fordart-sass
compatibilityWhat has not been added from Formation?
The CSS Library already offers utilities and tokens that we will want to use instead of porting over the old stuff:
What's next after this PR is merged?
related department-of-veterans-affairs/vets-design-system-documentation#2537
closes department-of-veterans-affairs/vets-design-system-documentation#2768
Acceptance criteria
Definition of done