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

fix(v2): navbar dropdown crash when item.to is undefined #3662

Merged
merged 4 commits into from
Nov 2, 2020
Merged

fix(v2): navbar dropdown crash when item.to is undefined #3662

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4972f12
Fix for undefined path with dropdown menu in Navbar
artemkovalyov Oct 30, 2020
571583b
Filter out 'href' links and check for undefined in utils
artemkovalyov Nov 1, 2020
d88d4da
Minot syntax fixes
artemkovalyov Nov 1, 2020
1724fee
allow isSamePath to accept undefined values
slorber Nov 2, 2020
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
2 changes: 1 addition & 1 deletion packages/docusaurus-theme-classic/src/theme/NavbarItem/DefaultNavbarItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function NavItemMobile({
const menuListRef = useRef<HTMLUListElement>(null);
const {pathname} = useLocation();
const [collapsed, setCollapsed] = useState(
() => !items?.some((item) => isSamePath(item.to, pathname)) ?? true,
() => !items?.filter((item) => item.to).some((item) => isSamePath(item.to, pathname)) ?? true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This now filters out all the navbar links with href instead of to so that we don't get undefined down the line. Path normalization is not required on external links I assume? Or instead of passing further item.to, you have to path just item and later check the path for to and href

);

const navLinkClassNames = (extraClassName?: string, isSubList = false) =>
Expand Down
4 changes: 2 additions & 2 deletions packages/docusaurus-theme-classic/src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@

// Compare the 2 paths, ignoring trailing /
export const isSamePath = (path1, path2) => {
const normalize = (str) => (str.endsWith('/') ? str : `${str}/`);
return normalize(path1) === normalize(path2);
const normalize = (str) => (str?.endsWith('/') ? str : `${str}/`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another safeguard for undefined. With such a fix it will work even without filtering done above and lead to a consistent Navbar menu structure.

return normalize(path1) === normalize(path2);
};