set subnav contains current item to false

[Hanskrogh]

Closes #4721

before-recording.mov

after-recording.mov

Changelog

New

Changed

1. Changed behavior of ItemWithSubNav within the NavList component.

Removed

Rollout strategy

• [ x] Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

⚠️ No Changeset found

Latest commit: de21ed4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[TylerJDev]

Thank you for the PR @Hanskrogh! This looks great ✨! I'm wondering, would it be possible for us to add a test in NavList.test.tsx so that we ensure that we always set containsCurrentItem to false when it is no longer the current item?

[Hanskrogh]

Thank you for the PR @Hanskrogh! This looks great ✨! I'm wondering, would it be possible for us to add a test in NavList.test.tsx so that we ensure that we always set containsCurrentItem to false when it is no longer the current item?

Hey @TylerJDev - I’ve added a snapshot test. It needed to be a snapshot test because the state only affects the ::after pseudo-element. I had to capture the initial styles and perform a couple of rerenders to ensure the component’s styles return to the initial state. 😊

Feel free to adjust as needed!

[TylerJDev]

Thanks a ton @Hanskrogh! I'll take a look at the changes before the week is up! Thank you again for the PR! ✨

[Hanskrogh]

Thanks a ton @Hanskrogh! I'll take a look at the changes before the week is up! Thank you again for the PR! ✨

Any news? :)

[TylerJDev]

I'm sorry for the delay @Hanskrogh! I'm coming at this a bit late, and now I'm having trouble reproducing this. Is/was there a page or example you tested this on? I followed the steps in #4721 but I suspect I'm missing a step.

Other than that and the comment I left, I think the only thing this PR would need is a changeset!

[TylerJDev]

I see the <ThemeProvider> was removed here and was just curious if we removed it intentionally? 🤔

[Hanskrogh]

@TylerJDev It was not intentionally ;)

[Hanskrogh]

@TylerJDev It seems like the npx changeset only allows me to create a Major release... I do not believe this is a major release :)