-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[usa-footer]: Preserve existing heading structure #5044
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
Conversation
Apologies for opening the PR too early. Ready for review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paulathevalley thanks for the contribution!
Just a few questions:
- This allows users of big footer to use any heading element by simply updating header in
.usa-footer__primary-link
, right? For example, if I wanted to useh6
instead ofh4
I'd have something like:<h6 class="usa-footer__primary-link">
. - Would you be able to add a test for this feature in footer.spec.js?
@mejiaj To answer your questions:
Thank you for your time! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you so much @paulathevalley!
I've tested:
- Footer can take any header, value gets saved as data attr on mobile and restored on desktop
- Unit test passes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement, thank you!
Description
Issue: Headings are not properly nested in the footer on OSG priorities pages. The
usa-footer/src/index.js
file assumes the footer headings will always beh4
elements.This PR sets a
data-tag
attribute with the html tag on the newbutton
element and reference it when toggling back to the original html tag.Additional information
Before you hit Submit, make sure you’ve done whichever of these applies to you:
npm test
and make sure the tests for the files you have changed have passed.(👋 first PR! hello! thank you for your patience)