Skip to content
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

Allow NavList selection/deselection by id/href in javascript #1822

Merged
merged 9 commits into from
Feb 21, 2023

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Feb 10, 2023

Description

We currently have logic in dotcom for automatically selecting NavList items when navigating via turbo. A lot of this logic depends on the internal HTML structure of the NavList, which is probably going to break in the near future when I start addressing a11y feedback. Moreover, the existing dotcom behavior doesn't properly expand the parent section if the selected item is a sub item of a section. This PR addresses both concerns by exposing a JavaScript API for selecting items by id, href, or the current URL. A significant portion of this code was copied and tweaked from dotcom.

It would be cool to upstream the turbo logic too, but selecting by id relies on setting a <meta> tag in the page header, which IMHO isn't something PVC should do.

Integration

Does this change require any updates to code in production?

No, dotcom code should continue to work as expected since none of the underlying HTML is changing. However once this is shipped I will submit a dotcom PR to call our new JavaScript API functions, which will obviate a good deal of dotcom code.

Merge checklist

  • Added/updated tests

- [ ] Added/updated documentation
- [ ] Added/updated previews

@changeset-bot
Copy link

changeset-bot bot commented Feb 10, 2023

🦋 Changeset detected

Latest commit: 852528b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@camertron camertron temporarily deployed to review-pr-1822 February 10, 2023 00:07 — with GitHub Actions Inactive
@github-actions github-actions bot added javascript Pull requests that update Javascript code ruby Pull requests that update Ruby code labels Feb 10, 2023
@camertron camertron temporarily deployed to github-pages February 10, 2023 00:11 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to review-pr-1822 February 10, 2023 05:34 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 10, 2023 05:39 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 10, 2023 19:29 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 10, 2023 19:54 — with GitHub Actions Inactive
@camertron camertron marked this pull request as ready for review February 10, 2023 20:02
@camertron camertron requested review from a team and keithamus February 10, 2023 20:02
@camertron camertron temporarily deployed to github-pages February 10, 2023 21:31 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 10, 2023 21:41 — with GitHub Actions Inactive
@camertron camertron requested a deployment to review-pr-1822 February 10, 2023 21:44 — with GitHub Actions Abandoned
@camertron camertron temporarily deployed to github-pages February 10, 2023 21:44 — with GitHub Actions Inactive
@camertron camertron removed the request for review from keithamus February 10, 2023 22:31
@camertron camertron temporarily deployed to review-pr-1822 February 10, 2023 23:03 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 10, 2023 23:03 — with GitHub Actions Inactive
@jonrohan jonrohan requested review from keithamus and removed request for jonrohan February 13, 2023 23:59
Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Not sure if @keithamus should look at this one

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me 👍

@camertron camertron temporarily deployed to review-pr-1822 February 15, 2023 23:19 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 15, 2023 23:22 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants