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

Double navbar after swizzling #3135

Closed
martavis opened this issue Jul 27, 2020 · 7 comments
Closed

Double navbar after swizzling #3135

martavis opened this issue Jul 27, 2020 · 7 comments
Labels
closed: working as intended This issue is intended behavior, there's no need to take any action.

Comments

@martavis
Copy link

🐛 Bug Report

After swizzling the navbar, I'm hit with useThemeContext is used outside of Layout component if I try using it without the Layout component. However, using the Layout component produces a navbar already.

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. Install Docusaurus v2 (alpha 59)
  2. Swizzle navbar
  3. Import navbar using @theme/NavBar into src/index.js
  4. Add navbar into Layout component - two navbars will render
  5. Remove Layout component but leave swizzled navbar - The page will not render due to the error useThemeContext is used outside of Layout component

Expected behavior

I expected the page to render as it did in previous builds. In addition, I expect to be able to import NavBar without needed to use the Layout component in order to use a custom navbar on custom pages. Or I expected to use the Layout component and my custom Navbar together.

Actual Behavior

Without Layout component:

image

With Layout component:

image

Your Environment

  • Docusaurus version used: v2 alpha 59
  • Environment name and version (e.g. Chrome 78.0.3904.108, Node.js 10.17.0): Windows Edge (Chromium) v 84.0.522.44
  • Operating system and version (desktop or mobile): Desktop

Reproducible Demo

https://github.com/martavis/docusuaurus-test

@martavis martavis added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Jul 27, 2020
@slorber
Copy link
Collaborator

slorber commented Jul 28, 2020

It's not clear to me what is your actual goal.

If you want to customize the navbar, you don't need to import it in your homepage, it's already included by the layout, so you just need to tweak your swizzled navbar.

Please tell me what you are trying to achieve

@martavis
Copy link
Author

martavis commented Jul 28, 2020

@slorber The issue is that the layout is not respecting the changes I have made. For example, if I comment out the block from line 132-139 on the swizzled NavBar, I expect to see the dark mode toggle disappear. That is not the result.

@slorber
Copy link
Collaborator

slorber commented Jul 28, 2020

@martavis so why report a totally different bug than the one you actually have in practice? 🤪

Editing a swizzled component should work if it's not the case please report this bug appropriately, this has nothing to do with double navbars or wrong hooks usage

First: we have a config option to disable this toggle. If it's your goal to remove the toggle, why don't you use it? (and if its not your goal, then what it is exactly? tell me the end result you expect by swizzling the navbar component)

Second: the code here does not comment the toggle. If you want my help, I want to see the code, and the result (site url or screenshot), and why you think it's a bug
https://github.com/martavis/docusuaurus-test/blob/master/src/theme/NavBar/index.js#L132

@martavis
Copy link
Author

@slorber Thank you for your help and quick replies. I reported what I thought was a bug, so I apologize because I'm not trying to waste anyone's time.

Commenting out the toggle was only meant to show that the swizzle wasn't respected. That is not my actual goal. My goal is to switch links depending on login status. So if the user is logged in, show different nav links than if not logged in. But for the purpose of this Github issue, I'm keeping the use-case simple.

To go a little more extreme, I commented out the entire right section of the nav starting at line 128.
https://github.com/martavis/docusuaurus-test/blob/master/src/theme/NavBar/index.js#L128

With lines 128 - 144 commented out, I expect to see no items on the right side of the navbar. Unfortunately, I still see them.

image

Does this seem like a valid bug or have I misinterpreted something? If the bug is valid, I will close this issue and create a new one.

@slorber
Copy link
Collaborator

slorber commented Jul 28, 2020

Hey @martavis , I almost went crazy with your repo 🤪

The problem is that you swizzled NavBar instead of Navbar, so you end up with the wrong case on your theme folder, and Webpack does not find it because it is case sensitive.

I've opened another issue with a better description so that we can find a solution to avoid this kind of problems: #3148

Thanks for reporting it.


Also thanks for sponsoring me on Github, you are the first one 😍

@slorber slorber closed this as completed Jul 28, 2020
@martavis
Copy link
Author

martavis commented Jul 28, 2020

@slorber I'm so mad at myself right now, thank you so much! I've been using Docusaurus for a couple of months now so I must have confused myself a while ago. Before the useContextTheme error was introduced, using the capital "B" was working fine, and SearchBar has a capital "B" as well...but I also imported my swizzled component manually so it didn't matter.

Anywho, thank you very much and you are well worth the sponsor! I'm truly grateful for people like you building and maintaining these types of tools.

@slorber
Copy link
Collaborator

slorber commented Jul 28, 2020

thanks ❤️

definitively not your fault, we should prevent this kind of mistakes, it also took me a while to figure this out.

I noticed the same item appeared twice in the webpack aliases, that seemed weird 🤪

"@theme/Navbar": "/Users/sebastienlorber/Desktop/projects/docusaurus/packages/docusaurus-theme-classic/lib/theme/Navbar/index.js",
"@theme/NavBar": "/Users/sebastienlorber/Desktop/projects/docusaurus/website/src/theme/NavBar/index.js",

@Josh-Cena Josh-Cena added closed: working as intended This issue is intended behavior, there's no need to take any action. and removed bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: working as intended This issue is intended behavior, there's no need to take any action.
Projects
None yet
Development

No branches or pull requests

3 participants