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

USWDS - Footer: Use :where() on all grid layout classes defined component styles #5749

Closed
wants to merge 1 commit into from

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Jan 29, 2024

Summary

Resolves style specificity issues between usa-footer and usa-grid-layout. This resolves visual regressions potentially caused by #5289.

Breaking change

This is not a breaking change.

Related issue

Closes #5676
Changelog → pending

Related pull requests

Grid dependency removed in #5289

Preview link

Footer →

Demo repo →

Demo footer →

Warning

By default, the test repo and preview are using 3.7.1. To see fix in affect, install this branch as USWDS

npm install @uswds/uswds "https://github.com/uswds/uswds/tree/cm-footer-grid-where-hotfix" --save

Problem statement

Layout grid styles defined in footer have higher specificity than other layout-grid styles. As a result, updated layout grid utility classes in footer markup do not affect layout as expected.

Solution

Use :where() to negate specificity in nested grid classes.

Testing and review

The demo repo below showcases how footer-defined grid styles have specificity over standard grid classes when both are imported to a project and then instructs the user to install this branch to resolve the issue.

  1. Checkout and start this test repo on your local machine
    • By default, this test repo uses 3.7.1
    • Imports both footer and grid-layout
  2. Inspect markup for both footers
    • Top footer features a mobile utility class which is found in footer and a desktop utility class found in grid-layout
    • Bottom footer features two utility classes, neither of which are included in footer css
  3. In desktop width viewports
    • Note that the top footer breaks only one link to second line.
    • The bottom footer has all links in one column.
  4. Inspect <li> style changes
    • Note that the footer defined classes take priority, despite layout-grid being included
  5. Install this branch
    • npm install @uswds/uswds "https://github.com/uswds/uswds/tree/cm-footer-grid-where-hotfix" --save
  6. Trigger sass rebuild
  7. Hard refresh the page
  8. Resize window and view changes
    • Note that both footers now break links into two columns with two rows on desktop widths
  9. Inspect styles at breakpoints
  10. Layout grid classes should now apply as expected

Testing checklist

  • Layout grid utility classes work as expected
  • Footer styles no longer take priority
  • Confirm there is no issue with non-nested gird classes in _usa-footer.scss
  • $theme-layout-grid-use-important: true is not required for desired layout-grid styles
  • Confirm :where() is needed on each grid class

Further consideration

I tested some of the options offered by sass:meta to test for the inclusion of the layout grid package. Since the grid classes are always included in our sass package, I was unable to use the functions to check for the existence of layout grid (They always return true).

We should consider if there are other ways to check for the inclusion of the package on the project level.

From @mejiaj in #5675

Long term, I like the idea of a flag to check if layout grid is loaded. We'll have to see what additional components rely heavily on layout grid.

@mahoneycm mahoneycm marked this pull request as ready for review January 30, 2024 18:44
@mahoneycm mahoneycm changed the title Use :where() on all grid layout classes defined in footer.scss USWDS - Footer: Use :where() on all grid layout classes defined component styles Jan 30, 2024
@mahoneycm mahoneycm changed the title USWDS - Footer: Use :where() on all grid layout classes defined component styles USWDS - Footer: Use :where() on all grid layout classes defined component styles Jan 30, 2024
@mejiaj
Copy link
Contributor

mejiaj commented Feb 6, 2024

@mahoneycm we discussed this in 02/06 Dev sync - Release check-in 🔒 (only GSA emails).

We're going with the other solution because, while more verbose, the browser support is better than :where().

@mahoneycm
Copy link
Contributor Author

mahoneycm commented Feb 8, 2024

Closing in favor of #5675 based on Dev Sync discussion

@mahoneycm mahoneycm closed this Feb 8, 2024
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.

USWDS - Bug: Removing layout-grid dependency causes style conflicts.
2 participants