USWDS - In-page navigation: Add custom headings attribute#5444
Conversation
…age-nav-header-select
| 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" |
There was a problem hiding this comment.
Removed these attributes so that the component uses defaults. The aim was to improve clarity in testing.
mahoneycm
left a comment
There was a problem hiding this comment.
Loving this change! Great work 👍
- Attribute name is intuitive and follows convention
- Correct headings are included
- Links are styled appropriately
- Test various heading levels in sandbox
-
h2 h3 h4 -
h2 h4 -
h4 h2 -
h3
-
- Confirmed highest level heading is respected despite order in data attribute
- JS matches USWDS standard
- Automated tests pass
|
Following |
- 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
There was a problem hiding this comment.
Note
Standardized the visual presentation of the expectations/instructions in this test to match the custom header test.
|
@mejiaj I believe I have addressed all of your questions/comments. Please let me know if you need anything else. |
mejiaj
left a comment
There was a problem hiding this comment.
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.
|
@mejiaj Thanks for catching these. Here are my responses to your items:
Let me know if you see anything else. |
mejiaj
left a comment
There was a problem hiding this comment.
Dismissing previous review because issue with empty nav is captured in #5610.
thisisdano
left a comment
There was a problem hiding this comment.
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
Summary
Added the ability to customize which headings will be pulled into the in-page navigation link list.
Use the new
data-heading-elementsattribute 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:
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-elementsattribute 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
data-heading-elementsattribute is presentTo test in Storybook:
To test in a project:
data-heading-elementsto any mix of headings and confirm that the link list pulls the correct headings.