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

Unify colors and add styles docs #2069

Merged
merged 22 commits into from
Jan 5, 2024
Merged

Unify colors and add styles docs #2069

merged 22 commits into from
Jan 5, 2024

Conversation

haworku
Copy link
Contributor

@haworku haworku commented Nov 20, 2023

Summary

  • Shared color and container tokens for use across the application. Use $mcr-* colors everywhere
  • Prefer @use to @import in module scss files and move towards namespaced imports
  • MCR Colors storybook component to make colors in use more visible
  • Add documentation how on to use styles and module scss files
  • Add stylelint and enforce no hex codes rule

Note: There is a new major storybook version (v7) from early 2023 that we should move to eventually. Seemed out of scope for this effort. I imagine we would port the Color storybook over tp e color palette format at that time. For now just copied spaceforce code for our Colors.stories.tsx

Related Issue

https://qmacbis.atlassian.net/browse/MCR-3847

Screenshots

Screenshot 2023-11-20 at 5 21 46 PM

QA guidance

  • Read the new doc
  • Take a look at Colors storybook
  • Run through the application, check each page of the application and compare with whats on VAL - ensure there are no regressions with spacing or colors specifically. Jira ticket has more detailed instructions.
    • If you find style regressions, please take side by side screenshots and comment them. Thank you!

@haworku haworku changed the title [DO NOT MERGE] Unify colors and add styles docs [REVIEW WELCOME, DO NOT MERGE] Unify colors and add styles docs Nov 20, 2023
@haworku haworku changed the title [REVIEW WELCOME, DO NOT MERGE] Unify colors and add styles docs REVIEW WELCOME, DO NOT MERGE: Unify colors and add styles docs Nov 20, 2023
@haworku haworku changed the title REVIEW WELCOME, DO NOT MERGE: Unify colors and add styles docs Unify colors and add styles docs Dec 18, 2023
@haworku
Copy link
Contributor Author

haworku commented Dec 19, 2023

I have a known issue where custom stylesheets is loaded multiple times on certain pages because of imports resolved in 2b21df2

-  delete dupe mcr-primary-lightest
- add -cyan-lighter so info alert banner color included
- add more foundation colors (focus, visited)
- add more lighter golds used in alerts and banners
@haworku
Copy link
Contributor Author

haworku commented Dec 29, 2023

@macrael @pearl-truss @JasonLin0991 Still waiting for reviews on this from an engineer. Design has taken a look. There are QA steps that I think make this manageable to review. Would like to land this early next week.

[class^='usa-step-indicator__header'] {
display: inline;
/* ACCESSIBILITY */
// Re-useable classname for hiding UI visually but still having it be read by screenreaders
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment is for line 61

Copy link
Contributor

@pearl-truss pearl-truss left a comment

Choose a reason for hiding this comment

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

I left and then deleted comments related to files having custom and or uswds imports that might not be necessary because it's probably beyond the scope of this PR

I also left some comments on the documentation but overall this looks really good! I really like the use of namespaces for imports and it's much clearer to me what is coming from where

docs/technical-design/howto-style-css-scss.md Outdated Show resolved Hide resolved
docs/technical-design/howto-style-css-scss.md Outdated Show resolved Hide resolved

The `styles/custom.scss` is also the main file imported into components to reference global SCSS variables. This file `@forward`s the MCR color palette and USWDS sass variables to the rest of the application.

### `USWDS` is the main design system in use
Copy link
Contributor

Choose a reason for hiding this comment

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

Visually this appears under the "How to Import" heading, is that intentional? I think there's a case for this being broken into its own section

@@ -0,0 +1,14 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't been able to get this to work. I enabled vscode-stylelint to test this, are you using something different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was running stylelint in app-web manually npx styelint **/*.scss- let me surface that as a yarn command also and document. It should be hooked into lint-staged as well (precommit script) I'll double check that.

@haworku
Copy link
Contributor Author

haworku commented Jan 3, 2024

I left and then deleted comments related to files having custom and or uswds imports that might not be necessary because it's probably beyond the scope of this PR

Appreciate that note though. I did just use across every file as you noticed with find and replace - not really caring if its actually used or not. I can clean up that up once this lands in between sprint work.

Copy link
Contributor Author

@haworku haworku Jan 5, 2024

Choose a reason for hiding this comment

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

Diff in this file is due to addressing a skipped test that failed lint. I deleted them since they aren't needed. Then prettier ran so things shifted around a bit

@pearl-truss
Copy link
Contributor

Thanks for making those changes! And I see the stylelint working on precommit now 🎉

@haworku haworku merged commit 33e0350 into main Jan 5, 2024
27 checks passed
@haworku haworku deleted the styles-docs branch January 5, 2024 17:49
@haworku haworku mentioned this pull request Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants