Visitar URL original
[usa-footer]: Preserve existing heading structure by paulathevalley · Pull Request #5044 · uswds/uswds · GitHub
Skip to content

[usa-footer]: Preserve existing heading structure - #5044

Merged
thisisdano merged 6 commits into
uswds:developfrom
paulathevalley:develop
Feb 28, 2023
Merged

thisisdano merged 6 commits into
uswds:developfrom
paulathevalley:develop

Conversation

@paulathevalley

@paulathevalley paulathevalley commented Nov 16, 2022 •

Copy link
Copy Markdown
Contributor

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 be h4 elements.

This PR sets a data-tag attribute with the html tag on the new button 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:

(👋 first PR! hello! thank you for your patience)

@paulathevalley
paulathevalley marked this pull request as draft November 16, 2022 21:05
@paulathevalley
paulathevalley marked this pull request as ready for review November 18, 2022 02:42
@paulathevalley

Copy link
Copy Markdown
Contributor Author

Apologies for opening the PR too early. Ready for review now.
cc @jkjustjoshing

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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:

  1. 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 use h6 instead of h4 I'd have something like: <h6 class="usa-footer__primary-link">.
  2. Would you be able to add a test for this feature in footer.spec.js?

@paulathevalley

Copy link
Copy Markdown
Contributor Author

@mejiaj To answer your questions:

  1. Yes, exactly.
  2. Yes, can do!

Thank you for your time!

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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

@mejiaj mejiaj added this to the uswds 3.4.0 milestone Dec 22, 2022
@mejiaj mejiaj added Added during Sprint Historical record of work added during a sprint. Not required for current triage. Package: Footer Work affecting Footer. Apply only when this area helps route or find the issue. labels Dec 22, 2022
@mejiaj
mejiaj requested a review from thisisdano January 4, 2023 21:53

@thisisdano thisisdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice improvement, thank you!

@thisisdano
thisisdano merged commit 1589158 into uswds:develop Feb 28, 2023
@thisisdano thisisdano mentioned this pull request Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Added during Sprint Historical record of work added during a sprint. Not required for current triage. Package: Footer Work affecting Footer. Apply only when this area helps route or find the issue.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants