docsifyjs / docsify

🃏 A magical documentation site generator.
https://docsify.js.org
MIT License
27.84k stars 5.68k forks source link

fix: fix loadSider false render structure issue #2470

Closed Koooooo-7 closed 4 months ago

Koooooo-7 commented 4 months ago

Summary

Enhance fix #519 .

Main fix

When the loadSidebar=false, it uses the toc to render auto-generated sidebar. But it puts the children nodes contents out of the parent node <li> block, which is mismatch the behavior with loadSidebar=true.

Minor changes

Refine the sidebar render part to always trigger by _renderMain callback instead of mess it in #renderMain. Now, the renderSidebar has the consistency that always being a callback after #renderMain no matter it load on loadSidebar=true/false.

Related issue, if any:

What kind of change does this PR introduce?

Bugfix

For any code change,

Does this PR introduce a breaking change?

Yes No

Tested in the following browsers:

vercel[bot] commented 4 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 28, 2024 8:15am
jhildenbiddle commented 4 months ago

@Koooooo-7 --

Tested this PR today. The nesting appears to be fixed (🥳), however a bug has been introduced where it appears that an attempt is made to load a sidebar file named null.md even when loadSidebar: true. You can see this by looking at the network tab of your browser's dev tools. The failing e2e test also shows the error message appearing in the console:

       "beforeEach",
        "afterEach-async",
        "afterEach",
        "doneEach",
        "ready",
    +   "Failed to load resource: the server responded with a status of 404 (Not Found)",
Koooooo-7 commented 4 months ago

however a bug has been introduced where it appears that an attempt is made to load a sidebar file named null.md even when loadSidebar: true.

@jhildenbiddle ...nice catch. I got a silly mistake to re-load the sidebar twice. Fixed.

Koooooo-7 commented 4 months ago

@jhildenbiddle --- Fix the issue 1 and 3. ~The Issue 2, once we have a final decision I will update asap.~ Issue2, aligned to add on the '.sidebar-nav > ul`.