Skip to content

Conversation

mitchmoccia
Copy link
Contributor

This implements the feature detailed here:
#5030

This is an important one that will allow for the customization of what headings should be queried when the in-page nav is being dynamically built.

A developer will be able to easily add the data- attribute data-headings to the aside element in the HTML to define which headings get queried.

Some examples include:
data-headings="h2"
data-headings="h2 h3"
data-headings="h2 h3 h4"
data-headings="h1 h2"

The headings should be listed with a space in between.

NOTE: This is most likely needed ASAP for the USWDS documentation site since including all h3s in the query is problematic on some of the pages... It might be better to use data-headings="h2" so only h2s are built in the in-page nav. Let's discuss.

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.

Looks good overall, just a question on the new data-headings setting.

`${MAIN_ELEMENT} h2, ${MAIN_ELEMENT} h3`
);
const getSectionHeadings = (headings) => {
const headingList = headings.indexOf(" ") ? headings.split(" ") : headings;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this headings.indexOf(" ") to check for multiple values?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we validate any of the values passed into data-headings? Or at least return an error if a user tries to pass an invalid header ex: j1 j2 instead of h1 h2 ?

For example, validate against a list of expected values? Could be a const VALID_HEADINGS = ["h1", "h2", "h3", "h4", "h5", "h6"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Outstanding suggestion, made it so, and it throws an error like the screen shot below if they use something other than a valid heading tag:

image

Copy link
Contributor Author

@mitchmoccia mitchmoccia Nov 10, 2022

Choose a reason for hiding this comment

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

Is this headings.indexOf(" ") to check for multiple values?

Yes, do you think this should include validation against, for example, if they use a dash instead of a space for multiple headings? We could do an IN_PAGE_NAV_VALID_DELIMITERS array like [" ", ",", "-"]?

Copy link
Contributor

Choose a reason for hiding this comment

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

That might not be necessary, but that's a good question. I feel like it's good enough to throw an error once the expected heading element is invalid.

FWIW; I had thought something similar like passing a value [h1, h2, h3] to the data-attribute for clarity. Unfortunately, it doesn't make things any easier for anyone.

@thisisdano
Copy link
Contributor

Holding on this one, pending further discussion

@amyleadem
Copy link
Contributor

Moving this work into #5444. Thanks for getting this started, @mitchmoccia!

@amyleadem amyleadem closed this Aug 17, 2023
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 - Feature: Add customizable data- attribute for in-page nav to specify which heading tags should be used
4 participants