-
Notifications
You must be signed in to change notification settings - Fork 0
BSD-492: Frontend style checks #495
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
Conversation
Add stylelinting for both drupal theme and storybook components.
Disabled the following: - `custom-property-pattern` because we're using private CSS vars to define component defaults - `selector-class-pattern` because we're using BEM syntax and I didn't want to add another dependency for it
…dentally show as successful if one passes.
…alidate action.
| "description": "", | ||
| "main": "gulpfile.js", | ||
| "scripts": { | ||
| "format:styles": "prettier './**/*.scss' --check", |
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.
Love this @mejiaj ! I didn't realize paths could be used in the action to only trigger when files in that dir change to run the action. I did a little more reading, and it seems if it's push it always run on the first because there's nothing to compare against. Makes me think about the validation I added in my commands, it runs regardless of changes in the branch. I'm not sure if I like the idea of a singular call that just does it all the time or a bunch of actions that have triggers on the paths they should be in.
What I wanted to ask was, are you sure we need this in the theme as well? It looks like your GitHub action runs only on the root, and that's configured so that it ignores stuff that shouldn't be run on in the theme already and overlaps the theme directories 100%.
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.
@mattsqd I see what you mean. I added it so developers could check if they were only working inside the Drupal theme.
I've removed in 54d7c19. I also added a note in the README for these new scripts.

https://github.com/Bixal/bixal-site-drupal/blob/54d7c19b81b6897b07ee6b58f997662947751f97/README.md#run-validation-checks
…matting. For now, we'll go with the top-level checks.

Summary
Add automated frontend formatting and linting checks and fix styles to comply. Keeps styles consistent in both Storybook and Bixal drupal theme.
Thanks to @levinmr, I used his work in Bixal/template-frontend#1 as a reference.
Related issue
Closes #492.
Solution
Lack of consistent style rules and CI checks allowed formatting drift across Storybook and the Drupal theme. This PR adds Prettier + Stylelint with CI checks so future changes are standardized and enforced.
Summary
Add automated frontend formatting and linting, and update SCSS to comply. Keeps USWDS styles consistent across Storybook and the Drupal theme.
Related issue
Closes #492
Solution
We lacked shared style rules and CI checks, which led to drift. This adds Prettier + Stylelint with a GitHub Action and aligns existing SCSS (syntax, mixins, selectors) so changes are standardized and enforced.
Major changes
format:styles,format:styles:fix,lint:styles,lint:styles:fix.stylelint-config-standard-scssandstylelint-prettier..prettierignoreand.stylelintignoreforstorybook-sass,dist,libraries,contrib,core,node_modules.rgb()with alpha,color-mix(), and lowercasecurrentcolor.::before/::after; use:where()/:is()where appropriate.custom-property-pattern: null).selector-class-pattern: null), prefer media query prefix notation.Testing and review
At root:
There shouldn't be any changes or errors.
In
web/themes/custom/bixal_uswds:There shouldn't be any changes or errors.
Visual smoke test (Storybook and/or Drupal build): Header, Footer, Hero, Buttons, Site Alert, People List, Emphasis Block. Check borders, shadows, icon fills, spacing, and list styles.
Confirm ignores work (no lint output from ignored directories).
For actions you should see
Frontend formatting & linting. It runs on push and PRs.The lint task relies on formatting, so it should fail if formatting fails.
On initial run formatting takes
20sand linting27s.Note
Checks will only run if SCSS changes.
Dependencies
Root package.json
bixal_uswds