Skip to content

Move component stylesheets to load dynamically with render#8571

Merged
aduth merged 11 commits intomainfrom
aduth-component-stylesheets
Jul 6, 2023
Merged

Move component stylesheets to load dynamically with render#8571
aduth merged 11 commits intomainfrom
aduth-component-stylesheets

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jun 9, 2023

🛠 Summary of changes

This builds upon #8375, shifting many of the existing stylesheets to be implemented as component-specific stylesheets. The benefits of this are outlined in #8375.

In practice, this means the application stylesheet is slightly smaller, instead loading many individual component stylesheets. There's a bit of a tradeoff here since the application stylesheet reduction is mitigated slightly by Brotli/DEFLATE's internal characteristics to reconcile duplication, which is not as effective in smaller, individual stylesheets. This would be most noticeable for components implemented in common layout or critical paths, e.g. language picker or Sign In page elements. Overall, I personally think it's still a net positive, all things considered.

This was originally going to include a refactor of LanguagePickerComponent to have a component stylesheet, but there's an issue with how we render the "stylesheet once" tags which is incompatible with components rendered as part of the base layout. This will need to be fixed first.

Performance Results

NODE_ENV=production yarn build:css && brotli-size app/assets/builds/application.css

Before: 26.9kb
After: 26.4kb
Diff: -0.5kb (-2%)

📜 Testing Plan

Ensure no regressions in the appearance of the affected components.

@aduth
Copy link
Contributor Author

aduth commented Jun 14, 2023

There's a small issue with the per-component stylesheets that I need to fix before merging this. Stylesheets which customize the font (example) are not outputting those styles when separated from the main application stylesheet, due to the optimization to avoid rendering font declarations in all except the main application stylesheet. It seems that USWDS detects that those fonts aren't configured and refuses to output the font style.

For example, in this branch, the OTP code entry field shows with normal text, but it's meant to show with monospace text.

@aduth aduth force-pushed the aduth-component-stylesheets branch from 7d87d88 to 99a2196 Compare June 30, 2023 14:59
@aduth
Copy link
Contributor Author

aduth commented Jun 30, 2023

There's a small issue with the per-component stylesheets that I need to fix before merging this. Stylesheets which customize the font (example) are not outputting those styles when separated from the main application stylesheet, due to the optimization to avoid rendering font declarations in all except the main application stylesheet. It seems that USWDS detects that those fonts aren't configured and refuses to output the font style.

For example, in this branch, the OTP code entry field shows with normal text, but it's meant to show with monospace text.

I think I found a resolution for this in 99a2196, which is a bit simpler overall as well.

@aduth aduth merged commit 87a779e into main Jul 6, 2023
@aduth aduth deleted the aduth-component-stylesheets branch July 6, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants