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

feat!: Uncouple uswds style #2532

Merged
merged 13 commits into from
Jan 18, 2024

Conversation

jpandersen87
Copy link
Contributor

@jpandersen87 jpandersen87 commented Jul 28, 2023

Summary

This PR removes the @forward 'uswds' line from the project's style index. This fixes the issue of uswds styles being included in the project's index.css file. Storybook's custom stylesheet has been updated to not reference the project's style index as the preview module is independently importing it along with uswds directly.

Related Issues or PRs

closes #2270

How To Test

  • build
  • verify that index.css does not contain uswds styles

@sawyerh
Copy link
Contributor

sawyerh commented Sep 12, 2023

This seems like a nice performance quick win, thanks for doing this @jpandersen87.

Copy link
Contributor

@kleinschmidtj kleinschmidtj left a comment

Choose a reason for hiding this comment

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

LGTM!

@brandonlenz
Copy link
Contributor

Has this been tested on a react-uswds consuming project?

Usually that's done by updating the project dependency to the head commit of this branch:

"dependencies": {
  "package": "git://github.com/username/package.git#commit"
}

@jpandersen87
Copy link
Contributor Author

Has this been tested on a react-uswds consuming project?

Usually that's done by updating the project dependency to the head commit of this branch:


"dependencies": {

  "package": "git://github.com/username/package.git#commit"

}

Not directly but for our project (ReportStream) I've been maintaining manual patches that matches the new 'index.css' output with success. We would get unwanted side effects when attempting to use a higher version of uswds than the package used due to the discrepancies between the uswds embedded in the index at default settings and our style sheet with customized settings (at the time it was unknown that this was the cause 😫).

Copy link
Contributor

@shkeating shkeating left a comment

Choose a reason for hiding this comment

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

does not appear to have design changes! good to go

@mdmower-csnw
Copy link
Contributor

mdmower-csnw commented Dec 10, 2023

Has this been tested on a react-uswds consuming project?

Usually that's done by updating the project dependency to the head commit of this branch:

"dependencies": {
  "package": "git://github.com/username/package.git#commit"
}

@brandonlenz

I'm still using v5 of @trussworks/react-uswds on a project so I cherry-picked these changes on top of v5.5.0 (see fork). My package.json dependency looks like this:

"@trussworks/react-uswds": "github:mdmower-csnw/react-uswds#5.5.0-uncouple-uswds-style",

The new size of node_modules/@trussworks/react-uswds/lib/index.css is only 3KB. The website still looks just like I expect, too.

@brandonlenz brandonlenz changed the title fix: Uncouple uswds style feat!: Uncouple uswds style Dec 27, 2023
@brandonlenz
Copy link
Contributor

Updated title to make this a major version change, since when this merges I want this to be noted as potentially breaking due to being a large enough change. Will make life easier for consumers if they run into challenges due to a different setup

@werdnanoslen werdnanoslen merged commit 6ce4bbb into trussworks:main Jan 18, 2024
7 checks passed
@brandonlenz
Copy link
Contributor

I see this was merged! Its a good change, I never did get around to testing it on one of our projects though. 🤞🏻 my hesitation is just paranoia

@jpandersen87 jpandersen87 deleted the uncouple-uswds-style branch January 18, 2024 18:27
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.

[fix] lib/index.css includes all USWDS styles, but should it?
8 participants