Shift header styles to navigation (account) stylesheet#10394
Conversation
changelog: Internal, Optimization, Reduce size of common application stylesheet
|
As I'm working through some of this refactoring, one thing I'm noticing about per-component or per-layout stylesheets is that it can be affected by some assumptions of ordering for CSS selectors of equal specificity, particularly when it comes to utility classes. Example:
We intentionally load utility classes late in the main application stylesheet to allow them to "win" in these scenarios, but since the application stylesheet as a whole is loaded before per-component or per-layout stylesheets, they still come before those other stylesheets. Some potential options:
|
|
Another thing I discovered in working on refactoring is that the event detail page in the account dashboard does not include mobile navigation menu, which causes a weird visual effect on the header that's present in production currently. There's no reason we couldn't include the mobile menu here and it simplifies the refactoring. Edit: I assume the reason we don't include the mobile menu here is because we don't include the side navigation on larger viewports, but (a) I don't think the inconsistency is unacceptable and (b) truthfully I don't understand why we don't show the side navigation on larger viewports 🤷
|
Stronger guarantees of associated style and JavaScript being loaded where needed Avoids extra computational considerations in application layout More consistent with USWDS-documented markup for header component occurring after banner Ensures consistent header appearance on event detail page
Prevent conflicts with equal specificity where utility classes in application stylesheet should be assumed to "win" See: #10394 (comment)
|
7c822e5 includes improved refactoring of rendering mobile navigation as part of the account page layout. The extended commit description details a number of benefits to this refactoring:
89c20d7 changes stylesheet load order to load the application stylesheet late, to address the issue described in #10394 (comment). I think it's reasonable to assume this should be loaded last to allow utilities contained within to take priority in cases of equal specificity. I'll do a quick pass over critical flows, but I believe this should be relatively safe. The only scenario I could imagine where this would be a problem is if the application stylesheet applies styles to individual components that we'd expect to "lose" in favor of styles in individual stylesheets in the case of equal CSS selector specificity. |
| <div class="usa-overlay"></div> | ||
| <% if content_for?(:mobile_nav) %> | ||
| <div class="usa-header"> | ||
| <%= yield(:mobile_nav) %> | ||
| </div> | ||
| <% end %> |
There was a problem hiding this comment.
Previously I might have assumed usa-overlay could be used for things other than mobile navigation (e.g. modals) and that's why it always needed to be present, but in reality it is very specifically for header/navigation.
See: https://github.com/search?q=repo%3Auswds%2Fuswds%20usa-overlay&type=code
Match application behavior See: #10394 (comment)
Match application behavior See: #10394 (comment)
* changelog: User-Facing Improvements, identity verification, Verified & Pending States for Identity Verification * add badges pertaining to verification status * add tooltip enabled component * add biometric verification to verified account view * add translated text * remove redundant key * change tooltip to use mouseover * clarify tooltip js and add testing * add spec for badge tooltip * move styling to badge tooltip * add readme to package * fix errors in yml * normalize yml * pinpoint countries * re-normalize yaml country codes * fix lint errors in badge scss's * remove unused keys * replace missing translation key * remove hardcoded app_name from translations * fix inconsistencies in verified button styling * remove unneeded css * add view tests for new badge placement * fix missing translated variable * adds tab event to tooltip. remove redundant status classes from badge border * refactor badges to incorporate an icon class * uncomplicate badge tab-into tooltip * add test coverage for changes to show_pii_partials? * direct help center articles links accordingly, fix conditional nesting in view * reformat formatted_due_date more succinctly * fix formatting regression with help center link, change tooltip to data- attribute * sync tooltip spec with data- attribute change * sync component spec with attribute change * remove data- prefix from tooltip-text * reset help_center links to use redirect * fix lint problems in erb file * possibly trailing space error affecting lint * mystery lint error * remove css class per change of style paradigm * add blank newline to end * lint error * Handle border and icon colors internal to BadgeComponent * Render stylesheet tag after component in preview Match application behavior See: #10394 (comment) * Sync badge component preview options * Remove unnecessary workaround for tooltip buttons See: https://github.com/18F/identity-idp/pull/10847/files#r1654791910 * Remove unused type: badge See: https://github.com/18F/identity-idp/pull/10847/files#r1654758970 * Assign tooltip classes internal to BadgeTooltipComponent * Add BadgeTooltipComponent preview * Render tag options on lg-badge-tooltip wrapper * Use default Tooltip component for tooltip behavior See: #10847 (comment) * Generalize BadgeTooltipCompponent as TooltipComponent * Use correct icons for verified badges * Handle additional IdV cases * Rearrange string keys for account verification content See: https://github.com/18F/identity-idp/pull/10847/files#r1654881240 * Include location parameters in help center redirect path help understand where the link was when the user clicked on it See: #10847 (comment) * Update specs * Fix spacing and indentation * Remove view rendering assertions from controller specs This is not a concern of the controller. View specs already validate this behavior. * reduce methods that are already memoized * replaced missing translation token * fix design system version dependency --------- Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
2 participants
🛠 Summary of changes
Moves header styles out of the critical-path stylesheet (
application.css) and into the dedicated stylesheet for the account page where the header is implemented (navigation.css).Why?
usa-search), since we only use the baseusa-headerclassPerformance Impact:
application.cssnavigation.css📜 Testing Plan
Verify there are no visual regressions in the header layout of either the base layout (e.g. "Sign in" page) or the account section pages, including mobile navigation.