Conversation
**Why**: So that, like with component-specific scripts, we can create stylesheet bundles which are loaded only on pages where the component is used, rather than as part of the single common "application" stylesheet, thereby reducing the size of the common stylesheet, and allowing for stylesheets to be broken up for parallel download. changelog: Internal, Optimization, Reduce size of common stylesheet
| 'widescreen': false, | ||
| ); | ||
|
|
||
| $line-height: 1.5; |
There was a problem hiding this comment.
This file organization is unsustainable, but I think we should embrace the unsustainability to force us to improve how these are managed: Namely, the typographical settings should be applied universally at the design system.
| def self.stylesheets | ||
| @stylesheets ||= sidecar_files_basenames(['scss']) |
There was a problem hiding this comment.
Technically, this behavior is unused, and is a remnant of early implementation approach. That being said, it feels like it's a reasonably expected behavior to align with how self.scripts is implemented, if we're going to support both self.scripts and self.stylesheets regardless (even if self.stylesheets would be implemented by the individual component).
| end | ||
|
|
||
| def render_assets | ||
| if helpers.respond_to?(:enqueue_component_scripts) && self.class.scripts.present? |
There was a problem hiding this comment.
I see it was in the old code, but can you remind me why we need to do a respond_to? check for that method?
There was a problem hiding this comment.
I see it was in the old code, but can you remind me why we need to do a
respond_to?check for that method?
A bit of speculative thinking that these components could be used outside Rails, based on #5398 (comment) , where enqueue_component_scripts would be optionally provided by the framework as a way to control how scripts (and now styles) are printed.
**Why**: Bit more conventional from condition usage site
| @@ -1,4 +1,5 @@ | |||
| @import 'identity-style-guide/dist/assets/scss/packages/required'; | |||
| @import '@18f/identity-idp-required-scss'; | |||
| @import '@18f/identity-components/troubleshooting-options'; | |||
There was a problem hiding this comment.
It feels a little fragile to have to pull in stylesheets based on the components being used in this pack. Also, this is duplicating the styles into the document-capture pack, rather than using the existing pack. I might want to think on this a bit more...
|
I probably won't have much capacity to revisit this in the near future. I'll plan to close the pull request, with the expectation that I'd like to resume it again in the future at some point. |
|
I'm interested to pick this up again, especially with USWDS v3's new component-specific imports. For example, this would have been nice to pair with #8307 as a way to include the USWDS "Tooltip" component only on pages where the clipboard button is used. |
2 participants
Why:
TroubleshootingOptionsComponentRails ViewComponent and correspondingTroubleshootingOptionsReact componentapp/assets/stylesheets, which is no longer necessaryDraft: