USWDS - Footer: Grid layout hotfix #5675

mahoneycm · 2023-12-13T22:21:08Z

Summary

Fixed a bug that caused some grid utility classes to be ignored when used inside usa-footer. 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

Related pull requests

Changes implemented in #5289

Preview link

Footer →

Demo repo →

Demo footer →

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

Tip

The files changed may look confusing, but it simply moves the styles above the .footer{} declaration instead of nesting them.

Move layout grid classes above .footer class.

Example

uswds/packages/usa-footer/src/styles/_usa-footer.scss

Lines 25 to 45 in 9fe48ce

    
           // Grid styles ported over to remove dependency 
        
           .grid-row { 
        
             @include grid-row; 
        
           } 
        
           // Gaps 
        
           .grid-gap { 
        
             @include grid-gap; 
        
           } 
        
           .grid-gap-1 { 
        
             @include grid-gap(1); 
        
           } 
        
           .grid-gap-2 { 
        
             @include grid-gap(2); 
        
           } 
        
           .grid-gap-4 { 
        
             @include grid-gap(4); 
        
           }

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.

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
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
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.
Inspect <li> style changes
- Note that the footer defined classes take priority, despite layout-grid being included
Install this branch
- npm install @uswds/uswds "https://github.com/uswds/uswds/tree/cm-footer-grid-hotfix" --save
Trigger sass rebuild
Hard refresh the page
Resize window and view changes
- Note that both footers now break links into two columns with two rows on desktop widths
Inspect styles at breakpoints
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

Further consideration

I considered investigating a sass:meta test to check for the inclusion of grid-layout and nest the footer grid classes within that but figured this would be enough for a hotfix/mvp.

Alternatively, we could investigate using a theme setting to enable/disable these styles. I opt for a logic test (if possible) so users are not required to take an extra step other than importing grid-layout

This restores setting from 3.7.0

mahoneycm · 2023-12-14T18:24:25Z

packages/usa-footer/src/styles/_usa-footer.scss

+  > .grid-container {
+    @include grid-container($theme-footer-max-width);


Moving .grid-container class to match what it was in 3.7.0 before changes were made.

uswds/packages/usa-footer/src/styles/_usa-footer.scss

Lines 26 to 34 in e670ce3

.usa-footer {

@include border-box-sizing;

@include typeset($theme-footer-font-family);

overflow: hidden;

> .grid-container {

@include grid-container($theme-footer-max-width);

}

}

@mahoneycm to confirm, the current state has them outside and this change brings them back under footer class?

The current state has all of the grid classes nested under the footer class. This change moves them outside of the footer class to resolve specificity issues.

I maintained .usa-footer > .grid-container to match 3.7.0

mejiaj

@mahoneycm a few questions:

Does this create duplicate styles in the final CSS file?
If so, is there a negative impact on performance?
Is there a method that can help us avoid specificity issues or duplicating styles (for example: :where() — baseline browser support or a flag to check if grid sass has loaded) ?

Here's how I understand the issues:

The specificity negatively impacts users.
Guidance is needed to direct users to load usa-layout-grid.

Is that correct?

mahoneycm · 2024-01-26T22:01:11Z

@mejiaj

Question response

1. Does this create duplicate styles in the final CSS file?

Yes

2. If so, is there a negative impact on performance?

It doesn't appear so
- develop - styles.css size: 640.05kb
- Fix - styles.css size: 639.56kb

3. Is there a method that can help us avoid specificity issues or duplicating styles (for example: `:where()` — baseline browser support or a flag to check if grid sass has loaded) ?

:where()
- Using :where() does seem to resolve style conflicts but it looks like we’ll have to use it on every grid class definition in footer.scss to negate specificity. How do you feel about that?
sass:meta
- I tried looking into a few sass:meta logic checks but since the classes are still being included in the uswds package the checks return true
- I believe a check like this would have to happen on the project level before being compiled into CSS.

Issues

1. The specificity negatively impacts users.

Precisely.
Ex.) If a user set a new desktop grid utility class, the mobile definition from footer would win the specificity check.

2. Guidance is needed to direct users to load `usa-layout-grid`.

Yes.
If users plan to use layout-grid classes that are not standard to our variants, they will need to load usa-layout-grid

mejiaj · 2024-01-29T17:52:51Z

@mahoneycm thanks for looking into my questions.

We should have guidance about the layout grid dependency on the website (if we don't have it already).

For immediate solutions, using :where() as a hotfix seems clearer in our intention to add layout support without needing to duplicate, override, or create specificity issues.

If you'd like to discuss with the team further please add the Discussion label and we can discuss in Slack or in Dev Sync.

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 · 2024-02-08T16:27:02Z

Reopened based on the discussion in Dev sync - Release check-in 🔒 (only GSA emails).

mejiaj

Thanks for re-opening this. Tested in DigitalGov and locally. No issues found.

I've modified the PR description to improve clarity on what changed by adding a code example.

amyleadem

Looking good! Just one small note about aligning .grid-gap output with the current CSS. Not sure if it is necessary but wanted to flag for consideration.

Confirm no visual regressions with default view
Confirm that various desktop grid classes work as expected
Confirm that various mobile-lg grid classes work as expected
Confirm that various tablet grid classes work as expected

packages/usa-footer/src/styles/_usa-footer.scss

amyleadem

Looks good to me!

mahoneycm added 3 commits December 13, 2023 13:20

Isolate grid-col classes

e9a9b07

Move footer styles outside of .footer class selector

85dde20

Move grid-container styles to be child of footer styles

9fe48ce

This restores setting from 3.7.0

mahoneycm mentioned this pull request Dec 14, 2023

USWDS-Sandbox - Demo for USWDS#5675 uswds/uswds-sandbox#121

Closed

mahoneycm commented Dec 14, 2023

View reviewed changes

mahoneycm requested review from mejiaj and amyleadem December 14, 2023 18:28

mahoneycm marked this pull request as ready for review December 14, 2023 18:29

mejiaj suggested changes Jan 26, 2024

View reviewed changes

mahoneycm mentioned this pull request Jan 30, 2024

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

Closed

5 tasks

This comment was marked as outdated.

Sign in to view

mahoneycm closed this Jan 30, 2024

mahoneycm reopened this Feb 8, 2024

mahoneycm requested a review from mejiaj February 8, 2024 16:32

This was referenced Feb 9, 2024

USWDS-Site: Documentation updates for release 3.8.0 uswds/uswds-site#2477

Closed

USWDS-Site - Changelog: Add entry for footer grid fix uswds/uswds-site#2484

Merged

mejiaj approved these changes Feb 9, 2024

View reviewed changes

amyleadem reviewed Feb 20, 2024

View reviewed changes

packages/usa-footer/src/styles/_usa-footer.scss Outdated Show resolved Hide resolved

mahoneycm added 2 commits February 20, 2024 16:48

Nest grid-gap styles within grid-row

a725a2b

Add & to join selectors

88a1130

mahoneycm requested a review from amyleadem February 21, 2024 15:29

amyleadem approved these changes Feb 21, 2024

View reviewed changes

amyleadem requested a review from thisisdano February 21, 2024 15:49

thisisdano approved these changes Feb 27, 2024

View reviewed changes

thisisdano merged commit b741317 into develop Feb 27, 2024

thisisdano deleted the cm-footer-grid-hotfix branch February 27, 2024 00:25

amyleadem mentioned this pull request Feb 28, 2024

USWDS-Site: Documentation updates for release 3.8.0 uswds/uswds-site#2509

Merged

amyleadem mentioned this pull request Mar 1, 2024

USWDS: Release 3.8.0 tasks #5797

Closed

8 tasks

thisisdano mentioned this pull request Mar 11, 2024

USWDS 3.8.0 #5812

Merged

mahoneycm mentioned this pull request May 6, 2024

USWDS - Bug: Banner & Footer style conflict caused by removing layout-grid dependency. #5914

Closed

2 tasks

mahoneycm mentioned this pull request May 15, 2024

USWDS - Footer: Restore layout-grid dependency #5930

Merged

SherfeyInv mentioned this pull request Jun 7, 2024

[Snyk] Upgrade @uswds/uswds from 3.7.1 to 3.8.0 SherfeyInv/digitalgov.gov#4

Closed

lane-formio mentioned this pull request Jun 22, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.1 formio/uswds-viewer#21

Closed

sennhann29 mentioned this pull request Jul 26, 2024

[Snyk] Upgrade @uswds/uswds from 3.4.1 to 3.8.1 sennhann29/digitalgov.gov#4

Open

lane-formio mentioned this pull request Aug 31, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.2 formio/uswds-viewer#25

Merged

FuhuXia mentioned this pull request Sep 26, 2024

[Snyk] Upgrade @uswds/uswds from 3.8.0 to 3.8.2 GSA/datagov-harvester#101

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

USWDS - Footer: Grid layout hotfix #5675

USWDS - Footer: Grid layout hotfix #5675

Uh oh!

mahoneycm commented Dec 13, 2023 •

edited by amyleadem

Loading

Uh oh!

mahoneycm Dec 14, 2023

Uh oh!

mejiaj Dec 18, 2023

Uh oh!

mahoneycm Dec 18, 2023

Uh oh!

mejiaj left a comment •

edited

Loading

Uh oh!

mahoneycm commented Jan 26, 2024 •

edited

Loading

Uh oh!

mejiaj commented Jan 29, 2024 •

edited

Loading

Uh oh!

This comment was marked as outdated.

mahoneycm commented Feb 8, 2024

Uh oh!

mejiaj left a comment •

edited

Loading

Uh oh!

amyleadem left a comment •

edited

Loading

Uh oh!

Uh oh!

amyleadem left a comment

Uh oh!

Uh oh!

	// Grid styles ported over to remove dependency
	.grid-row {
	@include grid-row;
	}

	// Gaps
	.grid-gap {
	@include grid-gap;
	}

	.grid-gap-1 {
	@include grid-gap(1);
	}

	.grid-gap-2 {
	@include grid-gap(2);
	}

	.grid-gap-4 {
	@include grid-gap(4);
	}

		> .grid-container {
		@include grid-container($theme-footer-max-width);

	.usa-footer {
	@include border-box-sizing;
	@include typeset($theme-footer-font-family);
	overflow: hidden;

	> .grid-container {
	@include grid-container($theme-footer-max-width);
	}
	}

USWDS - Footer: Grid layout hotfix #5675

USWDS - Footer: Grid layout hotfix #5675

Uh oh!

Conversation

mahoneycm commented Dec 13, 2023 • edited by amyleadem Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking change

Related issue

Related pull requests

Preview link

Problem statement

Solution

Testing and review

Testing checklist

Further consideration

Uh oh!

mahoneycm Dec 14, 2023

Choose a reason for hiding this comment

Uh oh!

mejiaj Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

mahoneycm Dec 18, 2023

Choose a reason for hiding this comment

Uh oh!

mejiaj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahoneycm commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Question response

1. Does this create duplicate styles in the final CSS file?

2. If so, is there a negative impact on performance?

3. Is there a method that can help us avoid specificity issues or duplicating styles (for example: :where() — baseline browser support or a flag to check if grid sass has loaded) ?

Issues

1. The specificity negatively impacts users.

2. Guidance is needed to direct users to load usa-layout-grid.

Uh oh!

mejiaj commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

mahoneycm commented Feb 8, 2024

Uh oh!

mejiaj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amyleadem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mahoneycm commented Dec 13, 2023 •

edited by amyleadem

Loading

mejiaj left a comment •

edited

Loading

mahoneycm commented Jan 26, 2024 •

edited

Loading

3. Is there a method that can help us avoid specificity issues or duplicating styles (for example: `:where()` — baseline browser support or a flag to check if grid sass has loaded) ?

2. Guidance is needed to direct users to load `usa-layout-grid`.

mejiaj commented Jan 29, 2024 •

edited

Loading

mejiaj left a comment •

edited

Loading

amyleadem left a comment •

edited

Loading