USWDS - In-page navigation: Add custom headings attribute #5444

amyleadem · 2023-08-16T22:52:54Z

Summary

Added the ability to customize which headings will be pulled into the in-page navigation link list.
Use the new data-heading-elements attribute to designate the heading levels that should be included in the component.

The headings should be listed with a space in between. Here is a sample implementation:

<aside 
    class="usa-in-page-nav" 
    data-heading-elements="h3 h4"
</aside>

If this attribute is not added to the component or has an empty value, it will default to pulling h2 and h3 headers.

Breaking change

This is not a breaking change.

Related issue

Closes #5030

Related pull requests

Changelog PR

This work originated in #5034.

Preview link

In-page navigation - custom header selector test

Problem statement

The in-page navigation component is hard-coded to pull all h2 and h3 headers from the designated main content region. However, there are valid use cases where teams would want to control which headings will be included in their component.

Solution

Creating data-heading-elements attribute will allow users select which header levels they want to include in the component link list. This attribute can accept multiple values (between h1-h6), each separated by a space. If the attribute is not added or is left blank, the component will pull the default h2 and h3 headers.

Testing and review

Confirm the attribute name is intuitive and follows convention
Confirm the correct headings are pulled when the data-heading-elements attribute is present
Confirm that bolded text styles are added to the top-level header in the link list. All other headers in the link list should receive a normal font weight.
Confirm code follows convention
Confirm the storybook controls make sense

To test in Storybook:

Open the In-page navigation - test custom header selector page
In the controls, choose different selections in the "Include these headers in link list" control. Reload the page after each selection change.
Confirm that only the expected headers are pulled into the in-page navigation link list.
1. "Default" should pull h2 and h3 headers
2. "All" should pull h1-h6 headers
3. "Error" should show an error in the console and not add any headers to the link list
4. "Empty" should pull h2 and h3 headers
5. Single header types should pull only the selected header type
Confirm that bolded text styles are added to the highest-level header in the link list. All other headers should have a normal font weight.

To test in a project:

Install this branch and copy the updated JS into your project

Add the following test markup

<div class="usa-in-page-nav-container">
  <aside class="usa-in-page-nav"
    data-heading-elements="h2">
  </aside>
  <main>
    <h1>H1 heading</h1>
    <h2>H2 heading</h2>
    <h3>H3 heading</h3>
    <h4>H4 heading</h4>
    <h5>H5 heading</h5>
    <h6>H6 heading</h6>
  </main>
</div>

Change the value of data-heading-elements to any mix of headings and confirm that the link list pulls the correct headings.

…age-nav-header-select

amyleadem · 2023-08-23T23:03:41Z

packages/usa-in-page-navigation/src/test/test-patterns/test-custom-content-selector.twig

-    data-title-text="On this page"
-    data-title-heading-level="h4"
-    data-scroll-offset="0"
-    data-root-margin="0px 0px 0px 0px"
-    data-threshold="1"


Removed these attributes so that the component uses defaults. The aim was to improve clarity in testing.

packages/usa-in-page-navigation/src/index.js

mahoneycm

Loving this change! Great work 👍

smustgrave · 2023-11-06T21:15:12Z

Following

mejiaj

Works well and the story controls were helpful! I was able to test:

All available settings
Changing order of headings data-heading-selector="h6 h5 h1 h2 h3 h4"
Defaults work as expected

Added some suggestions to improve clarity. Also, can you add the release note to the PR description?

packages/usa-in-page-navigation/src/styles/_usa-in-page-navigation.scss

packages/usa-in-page-navigation/src/test/test-patterns/test-custom-header-selector.twig

packages/usa-in-page-navigation/src/test/test-patterns/test-hidden-headers.twig

packages/usa-in-page-navigation/src/usa-in-page-navigation.stories.js

packages/usa-in-page-navigation/src/index.js

packages/usa-in-page-navigation/src/test/test-patterns/test-custom-header-selector.twig

packages/usa-in-page-navigation/src/index.js

mejiaj · 2024-01-05T16:18:05Z

packages/usa-in-page-navigation/src/index.js

@@ -216,9 +248,11 @@ const createInPageNav = (inPageNavEl) => {
    const anchorTag = document.createElement("a");
    const textContentOfLink = el.textContent;
    const tag = el.tagName.toLowerCase();
+    const topHeadingLevel = getTopLevelHeading(sectionHeadings);


Just noting this runs when there's only a single heading passed. This seems okay for the small amount of headers available.

- Rename Error control - Remove empty control since it is the same as "default" - Change control from inline-radio to select

- This adds clarity to the code

…age-nav-header-select

amyleadem · 2024-01-08T19:03:16Z

packages/usa-in-page-navigation/src/test/test-patterns/test-hidden-headers.twig

Note
Standardized the visual presentation of the expectations/instructions in this test to match the custom header test.

amyleadem · 2024-01-08T19:10:23Z

@mejiaj I believe I have addressed all of your questions/comments. Please let me know if you need anything else.

mejiaj

Looking good.

Just sound small outstanding items:

Note on BEM I missed on first round (sorry)
Please add the release note in the PR description with the bold text + description.
Old question in previous review.
Question on sanitization in JS.

packages/usa-in-page-navigation/src/index.js

packages/usa-in-page-navigation/src/styles/_usa-in-page-navigation.scss

amyleadem · 2024-01-09T23:04:15Z

@mejiaj Thanks for catching these. Here are my responses to your items:

Note on BEM I missed on first round (sorry)
- Fixed (related comment)
Please add the release note in the PR description with the bold text + description.
- Added
Old question in previous review.
- Answered question in this comment
Question on sanitization in JS.
- Fixed (related comment)

Let me know if you see anything else.

mejiaj

Looking good, minor note on nonexistent headers.

Testing items

Single header values like h1
Several headers like h1, h2, h3
~~Specifying headers that don't appear ex: h6. Empty nav is generated, see comment below.~~

packages/usa-in-page-navigation/src/index.js

mejiaj

Dismissing previous review because issue with empty nav is captured in #5610.

thisisdano

I'm going to approve this, but I think there's a modest usability issue here since there's no distinction between the "top" heading level, and all the "lower" heading levels. But that could be a issue for another day

amyleadem added 8 commits August 16, 2023 15:52

Add custom headings attribute

631e8b9

Create story for custom header

288e9eb

Remove attribute customizations from stories

219c8a1

Update control type for custom header selector

4406395

Add default, empty, and error states

566bc0f

Run prettier

e9b099f

Run prettier

7935c16

Add getTopLevelHeading()

44631cd

amyleadem mentioned this pull request Aug 17, 2023

In-page nav - Add customizable data- attribute for which heading[s] gets queried #5034

Closed

amyleadem added 3 commits August 17, 2023 11:14

Update data-headings --> data-heading-selector

2edf4df

Run prettier

3c042b2

Remove test code

95d8970

amyleadem mentioned this pull request Aug 17, 2023

USWDS-Site - Changelog: In-page nav header selector [#5444] uswds/uswds-site#2252

Merged

Base automatically changed from al-in-page-nav-parent to develop August 18, 2023 16:49

amyleadem linked an issue Aug 22, 2023 that may be closed by this pull request

USWDS - Feature: Add customizable data- attribute for in-page nav to specify which heading tags should be used #5030

Closed

2 tasks

amyleadem added 8 commits August 22, 2023 10:58

Simplify steps in getSectionHeadings

ca16ca3

Rename contentHeadings --> sectionHeadings

3e2626f

Format code

3e1cd94

Add clarity to control names

3014087

Create unit test

5d1a360

Update param type for selectedHeadingTypes

35bf710

Update test names

41493cb

Merge branch 'develop' of https://github.com/uswds/uswds into al-in-p…

9d9492a

…age-nav-header-select

amyleadem commented Aug 23, 2023

View reviewed changes

packages/usa-in-page-navigation/src/index.js Outdated Show resolved Hide resolved

amyleadem marked this pull request as ready for review August 23, 2023 23:07

amyleadem requested review from mejiaj and mahoneycm August 23, 2023 23:07

mahoneycm approved these changes Sep 18, 2023

View reviewed changes

mejiaj requested changes Jan 5, 2024

View reviewed changes

amyleadem added 8 commits January 5, 2024 10:37

Update test descriptions

9de0209

Define usa-in-page-nav__item with u-font-weight

03589a1

Clean up header twig; includeHeaders -> headingType

8eabccc

- Rename Error control - Remove empty control since it is the same as "default" - Change control from inline-radio to select

Add clarity to custom heading error message

d7d27a6

Separate getSectionHeadings into two functions

30a3b9e

- This adds clarity to the code

Change data-heading-selector -> data-heading-elements

3467c74

Merge branch 'develop' of https://github.com/uswds/uswds into al-in-p…

25e3a06

…age-nav-header-select

Run prettier

8282ace

amyleadem commented Jan 8, 2024

View reviewed changes

amyleadem requested a review from mejiaj January 8, 2024 19:10

mejiaj requested changes Jan 9, 2024

View reviewed changes

packages/usa-in-page-navigation/src/index.js Outdated Show resolved Hide resolved

packages/usa-in-page-navigation/src/index.js Outdated Show resolved Hide resolved

packages/usa-in-page-navigation/src/styles/_usa-in-page-navigation.scss Outdated Show resolved Hide resolved

amyleadem added 2 commits January 9, 2024 15:52

Replace nav__item--sub-item -> nav__item--primary

df47eab

Sanitize data-heading-elements

7a32ee8

amyleadem requested a review from mejiaj January 9, 2024 23:04

mejiaj requested changes Jan 22, 2024

View reviewed changes

packages/usa-in-page-navigation/src/index.js Show resolved Hide resolved

mejiaj approved these changes Jan 22, 2024

View reviewed changes

mejiaj requested a review from thisisdano January 22, 2024 21:06

mahoneycm mentioned this pull request Feb 1, 2024

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

Closed

5 tasks

thisisdano reviewed Feb 26, 2024

View reviewed changes

thisisdano approved these changes Feb 26, 2024

View reviewed changes

thisisdano merged commit baaf7f9 into develop Feb 26, 2024
5 checks passed

thisisdano deleted the al-in-page-nav-header-select branch February 26, 2024 23:31

This was referenced Feb 28, 2024

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

Merged

USWDS: Release 3.8.0 tasks #5797

Closed

thisisdano mentioned this pull request Mar 11, 2024

USWDS 3.8.0 #5812

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

USWDS - In-page navigation: Add custom headings attribute #5444

USWDS - In-page navigation: Add custom headings attribute #5444

amyleadem commented Aug 16, 2023 •

edited

amyleadem Aug 23, 2023 •

edited

mahoneycm left a comment

smustgrave commented Nov 6, 2023

mejiaj left a comment •

edited

mejiaj Jan 5, 2024

amyleadem Jan 8, 2024

amyleadem commented Jan 8, 2024

mejiaj left a comment

amyleadem commented Jan 9, 2024

mejiaj left a comment •

edited

mejiaj left a comment

thisisdano left a comment

USWDS - In-page navigation: Add custom headings attribute #5444

USWDS - In-page navigation: Add custom headings attribute #5444

Conversation

amyleadem commented Aug 16, 2023 • edited

Summary

Breaking change

Related issue

Related pull requests

Preview link

Problem statement

Solution

Testing and review

amyleadem Aug 23, 2023 • edited

Choose a reason for hiding this comment

mahoneycm left a comment

Choose a reason for hiding this comment

smustgrave commented Nov 6, 2023

mejiaj left a comment • edited

Choose a reason for hiding this comment

mejiaj Jan 5, 2024

Choose a reason for hiding this comment

amyleadem Jan 8, 2024

Choose a reason for hiding this comment

amyleadem commented Jan 8, 2024

mejiaj left a comment

Choose a reason for hiding this comment

amyleadem commented Jan 9, 2024

mejiaj left a comment • edited

Choose a reason for hiding this comment

mejiaj left a comment

Choose a reason for hiding this comment

thisisdano left a comment

Choose a reason for hiding this comment

amyleadem commented Aug 16, 2023 •

edited

amyleadem Aug 23, 2023 •

edited

mejiaj left a comment •

edited

mejiaj left a comment •

edited