-
Notifications
You must be signed in to change notification settings - Fork 67
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
Update documentation to remove formation and add css-library #3650
Conversation
@@ -57,23 +57,23 @@ To add content, you will need to look into `/src` directory. This will be the so | |||
|
|||
[Read the wiki](https://github.com/department-of-veterans-affairs/vets-design-system-documentation/wiki) to learn how to add new pages to design.va.gov, improve local search, add images, etc. | |||
|
|||
## Testing updates to the Formation codebase on this site | |||
## Testing updates to the CSS-Library codebase on this site | |||
|
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 just updated this copy and I'm not sure if there is a better way to give these instructions or if we need to update them.
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 I think there might be some things we need to confirm before we can give these instructions. I have a feeling that even the Formation instructions don't actually work any more.
I would lean towards removing these steps completely rather than say something that doesn't work. Should we create a follow-up ticket for writing new steps for testing? Or maybe just remove this and link to the component-library vets-website testing instructions?
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.
@powellkerry did you happen to test the steps below to see if they work for css library? I'm specifically interested in learning if that local version thing on line 74 works, since that seems to be the key piece for connecting the two repos for local testing.
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.
@micahchiang yes the instructions work, but it's not as easy as they imply. I had to remove the node_modules directory and run yarn in vets-design-system-documentation in order for any changes to show. I think updating the instructions to use verdaccio might be better?
@@ -46,7 +46,13 @@ The VA follows the USWDS spacing unit tokens and then adds additional semantic t | |||
|
|||
In order to keep spacing consistent throughout VA.gov, it is recommended you favor using the `units` functions instead of hard coding pixels (or relative units) for margins and padding. | |||
|
|||
In order to access the spacing tokens in your project, you will need to import the [base files](https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/tree/master/packages/formation/sass/base) into your project. [Here is how this site is doing that](https://github.com/department-of-veterans-affairs/vets-design-system-documentation/blob/main/src/assets/stylesheets/application.scss#L5). | |||
In order to access the spacing tokens in your project, you will need to import the [base files](https://github.com/department-of-veterans-affairs/component-library/tree/main/packages/css-library/src/stylesheets) into your project. | |||
<!-- |
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 removed this link to look at an example because the code shows code references to formation. I will leave a comment on the ticket for updating the code references to re-add this link.
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 piece about spacing tokens is a little confusing because the paragraph above references a units function 🤔
Can spacing tokens be accessed directly be importing https://github.com/department-of-veterans-affairs/component-library/blob/main/packages/css-library/dist/tokens/css/variables.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.
I updated this. I think the spacing tokens can be used by importing that variables file.
src/_about/developers/install.md
Outdated
|
||
``` | ||
$formation-asset-path: '../assets'; |
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 couldn't find a asset-path
variable in css-library. Should this be updated or removed?
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 not exactly sure where we are with the state of assets and the CSS Library. @micahchiang Do you have any thoughts on how we should update these steps for now?
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 for now, we should mark the existing stuff as deprecated, with a message that new guidance is coming soon. We should probably conduct our own spike or experiment into what is needed to get css-library working properly across a number of different projects. I can write a follow up ticket for this.
@@ -24,7 +24,7 @@ labels: platform-design-system-team, bug | |||
|
|||
## Reproducing | |||
|
|||
- Formation version: | |||
- CSS-Library version: |
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.
A little bit of a nit here but should the name of the library be referenced as CSS Library
throughout (sans dash)? I feel like this is one of those things that once we put it into stone (the documentation), it will forever be that way.
@@ -57,23 +57,23 @@ To add content, you will need to look into `/src` directory. This will be the so | |||
|
|||
[Read the wiki](https://github.com/department-of-veterans-affairs/vets-design-system-documentation/wiki) to learn how to add new pages to design.va.gov, improve local search, add images, etc. | |||
|
|||
## Testing updates to the Formation codebase on this site | |||
## Testing updates to the CSS-Library codebase on this site | |||
|
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 I think there might be some things we need to confirm before we can give these instructions. I have a feeling that even the Formation instructions don't actually work any more.
I would lean towards removing these steps completely rather than say something that doesn't work. Should we create a follow-up ticket for writing new steps for testing? Or maybe just remove this and link to the component-library vets-website testing instructions?
@@ -16,8 +16,7 @@ | |||
"keywords": [ | |||
"Vets.gov", | |||
"design", | |||
"uswds", | |||
"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.
👋🏼 byeeee
src/_about/developers/install.md
Outdated
@@ -11,24 +11,21 @@ anchors: | |||
|
|||
## Install CSS library into your project |
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.
Should we capitalize the "L" in library while we're here?
src/_about/developers/install.md
Outdated
|
||
``` | ||
$formation-asset-path: '../assets'; |
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 not exactly sure where we are with the state of assets and the CSS Library. @micahchiang Do you have any thoughts on how we should update these steps for now?
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.
Updates are looking good to me but do you mind checking the developer > install page again? https://dev-design.va.gov/3650/about/developers/install#install-css-library-into-your-project
Do we want to keep all of those references to Formation still even if we are planning to re-write the instructions? The header of the section is talking about CSS Library so it's a mix of libraries and confusing.
Do we have a ticket yet for discovering how to install CSS Library in different scenarios outside of vets-website? (Code Pen, Code Sandbox, etc)?
src/_about/developers/install.md
Outdated
permalink: /about/developers/install | ||
has-parent: /about/developers/ | ||
intro-text: | ||
intro-text: "New guidance in coming soon. With the deprecation of Formation and the adoption off CSS Library the design system team is in the process of creating and providing guidance on using CSS Library across different projects." | ||
anchors: | ||
- anchor: Install CSS library into your project |
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.
Capitalize the "L" in "CSS library".
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 did a global search and replaced all the lower-cased "l"s, nice to be consistent ;)
…on, capitalization semantics
…ent-of-veterans-affairs/vets-design-system-documentation into 3583-update-formation-references
@jamigibbs I updated install.md to say formation instead of css-library. I had just reverted my changes to that file and didn't notice that it mentioned css library. Evidently when this documentation was created, they used to refer to formation as the css library... I also created a ticket to research and rewrite this file: #3663 |
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.
Evidently when this was created, they used to refer to formation as the css library
Oh dear. 🫣
Update all references and links to formation with the correct information and links to css-library. This is needed due to formation being deprecated and no longer being used in vets-website.
Closes 3583
Note, this does not update code references to formation. That work is planned in this ticket