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.

Sign up for GitHub

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

Jump to bottom

set subnav contains current item to false #4722

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

set subnav contains current item to false #4722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ef5067
set subnav contains current item to false
Hanskrogh Jul 7, 2024
de21ed4
test(NavList): updated test to account for containsCurrentItem
Hanskrogh Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 64 additions & 13 deletions packages/react/src/NavList/NavList.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,20 +246,16 @@ describe('NavList.Item with NavList.SubNav', () => {

it('does not have active styles if SubNav contains the current item and is open', () => {
const {container, queryByRole} = render(
<ThemeProvider>
<SSRProvider>
<NavList>
<NavList.Item>
Item
<NavList.SubNav>
<NavList.Item href="#" aria-current="page">
Sub Item
</NavList.Item>
</NavList.SubNav>
<NavList>
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

@TylerJDev It was not intentionally ;)

Copy link
Author

Choose a reason for hiding this comment

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

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

<NavList.Item>
Item
<NavList.SubNav>
<NavList.Item href="#" aria-current="page">
Sub Item
</NavList.Item>
</NavList>
</SSRProvider>
</ThemeProvider>,
</NavList.SubNav>
</NavList.Item>
</NavList>,
)

// Starts open
Expand All @@ -269,6 +265,61 @@ describe('NavList.Item with NavList.SubNav', () => {
expect(container).toMatchSnapshot()
})

it('sets containsCurrentItem to false when the current item is changed and sub-navigation does not contain the current item', () => {
const {container, rerender} = render(
<NavList>
<NavList.Item>
Item 2
<NavList.SubNav>
<NavList.Item href="#">Sub Item 1</NavList.Item>
<NavList.Item href="#">Sub Item 2</NavList.Item>
</NavList.SubNav>
</NavList.Item>
<NavList.Item href="#" aria-current="page">
Item 1
</NavList.Item>
<NavList.Item href="#">Item 3</NavList.Item>
</NavList>,
)

// First rerender with a different current item
rerender(
<NavList>
<NavList.Item>
Item 2
<NavList.SubNav>
<NavList.Item href="#" aria-current="page">
Sub Item 1
</NavList.Item>
<NavList.Item href="#">Sub Item 2</NavList.Item>
</NavList.SubNav>
</NavList.Item>
<NavList.Item href="#">Item 1</NavList.Item>
<NavList.Item href="#">Item 3</NavList.Item>
</NavList>,
)

// Second rerender to revert to the initial state
rerender(
<NavList>
<NavList.Item>
Item 2
<NavList.SubNav>
<NavList.Item href="#">Sub Item 1</NavList.Item>
<NavList.Item href="#">Sub Item 2</NavList.Item>
</NavList.SubNav>
</NavList.Item>
<NavList.Item href="#" aria-current="page">
Item 1
</NavList.Item>
<NavList.Item href="#">Item 3</NavList.Item>
</NavList>,
)

// Capture the final state of the component
expect(container).toMatchSnapshot()
})

it('prevents more than 4 levels of nested SubNavs', () => {
const consoleSpy = jest
.spyOn(console, 'error')
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/NavList/NavList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ function ItemWithSubNav({children, subNav, depth, defaultOpen, sx: sxProp = defa
if (currentItem) {
setContainsCurrentItem(true)
setIsOpen(true)
} else {
// If the SubNav doesn't contain the current item, set it to false
setContainsCurrentItem(false)
}
}
}, [subNav, buttonId])
Expand Down
Loading