Skip to content

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Dec 3, 2024

Summary

Removed overflow: hidden from usa-footer to allow the full focus outline to show. This fix also improves alignment in the slim footer variant.

Note

This is a follow-on for PR #6093 (Thanks @6TELOIV!)

Breaking change

This is not a breaking change.

Related issue

Closes #6092

Related pull requests

Changelog

Preview link

Footer component

Problem statement

Focus rings are visually cut off in the footer. Additionally, the padding in the slim footer prevents the top border from extending the full width of the component (like it does in the other variants).
image

Solution

  1. Removing overflow: hidden allows focus rings to show at the edge of the footer.
  2. Removing the x padding from the slim footer mobile presentation resolves both of these issues.

Screenshots

Default footer

image

Slim footer

image

Note that this change also fixes the slim footer alignment in medium screen widths:

image

Testing and review

  • Footer no longer hides pieces of the focus outline
  • Confirm there are no visual regressions:
    • As a result of removing overflow: hidden
    • In link lists in all variants in mobile view
    • In link lists in all variants in desktop view
  • Confirm code meets USWDS convention

@@ -66,6 +65,7 @@ $-chevron-expand-more: map-merge(

.usa-footer__primary-container {
@include grid-container($theme-footer-max-width);
@include u-padding-x(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This is the only meaningful change from #6093. All other changes are just re-ordering the style rules to match convention.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good to me! Both issues are fairly related, so getting both done in one pull makes sense to me

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirming that all variants look good on all screen sizes. Thanks for building off of the good work from @6TELOIV.

@amyleadem amyleadem marked this pull request as ready for review December 4, 2024 17:50
@amyleadem amyleadem requested a review from a team as a code owner December 4, 2024 17:50
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! All three variants are much more uniform now.

Question: Should we set the target to #6093? Or are we going to close that one in favor of this one?

It might be good to keep the previous one due to the discussion and decisions captured there

@amyleadem amyleadem changed the title [Test] USWDS - Footer overflow focus USWDS - Footer: Fix overflow focus cutoff and padding Dec 5, 2024
@thisisdano thisisdano merged commit 7a2f8d4 into develop Dec 16, 2024
5 checks passed
@thisisdano thisisdano deleted the al-footer-overflow-focus branch December 16, 2024 22:14
@thisisdano thisisdano mentioned this pull request Dec 18, 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: Focus ring in footer links is cut off
5 participants