Skip to content

NavList: Fix when subNav opens automatically and shows current indicator - #3611

Merged
siddharthkp merged 2 commits into
mainfrom
navlist-current-fix-1
Aug 14, 2023
Merged

NavList: Fix when subNav opens automatically and shows current indicator#3611
siddharthkp merged 2 commits into
mainfrom
navlist-current-fix-1

Conversation

@siddharthkp

@siddharthkpsiddharthkp commented Aug 9, 2023

Copy link
Copy Markdown
Member

Before: If the second item is selected, NavList does not open the subNav automatically and does not show current indicator at the subNav level when folded

before-navlist.mov

After: If the second item is selected, NavList does open subNav automatically and shows the current indicator at the subNav level

after-navlist.mov

Implementation detail

While we did check for aria-current !== false, we did this for the first element with aria-current, which could be the one with aria-current=false :)

Changing the selector to exclude false value takes care of this.

- subNavRef.current.querySelector('[aria-current]')+ subNavRef.current.querySelector('[aria-current]:not([aria-current=false])')

@siddharthkpsiddharthkp added bug Something isn't working patch release bug fixes, docs, housekeeping react labels Aug 9, 2023
@siddharthkpsiddharthkp self-assigned this Aug 9, 2023
@siddharthkp
siddharthkp requested review from a team and broccolinisoupAugust 9, 2023 14:58
@changeset-bot

changeset-botBot commented Aug 9, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8b84b33

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

@siddharthkp
siddharthkp requested review from joshblack and removed request for broccolinisoupAugust 9, 2023 14:59
@github-actions

github-actionsBot commented Aug 9, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js103.64 KB (-0.01% 🔽)
dist/browser.umd.js104.21 KB (-0.01% 🔽)

@siddharthkp
siddharthkp added this pull request to the merge queueAug 14, 2023
Merged via the queue into main with commit ace603dAug 14, 2023
@siddharthkp
siddharthkp deleted the navlist-current-fix-1 branch August 14, 2023 16:02
This was referenced Aug 14, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't workingpatch releasebug fixes, docs, housekeepingreact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@siddharthkp@broccolinisoup@joshblack@langermank