Skip to content

Conversation

sy-records
Copy link
Member

Summary

When there is no sub sidebar, the sidebar should be closed.

Related issue, if any:

Close #2556

What kind of change does this PR introduce?

Refactor

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed

Does this PR introduce a breaking change?

No

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge
Copy link

vercel bot commented Jun 12, 2025

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 Jun 12, 2025 8:07am
@paulhibbitts
Copy link
Collaborator

paulhibbitts commented Jun 12, 2025

Hi @sy-records , thanks so much for looking into this!

Your fix looks really good, but before proceeding I thought I'd share two of those original demos updated with your preview build to confirm that everyone is on-board re: updated behaviour.

Here is the updated codebox (using your new build), tap on the second page topic and sidebar now closes on mobile:
https://codesandbox.io/p/sandbox/docsify-v5-sidebar-nav-ljxvyd

Here is an the updated Docsify v5 rc-1 example using your new build - tap on Topic One and Topic two when on mobile and now the sidebar closes as expected:
https://hibbitts-design.github.io/preview-docsify-open-publishing-starter-kit-v2-docsify-v5-theme/#/

Thanks again, rc-1 is really coming along 🚀

@paulhibbitts
Copy link
Collaborator

paulhibbitts commented Jun 13, 2025

I've done more tests and this continues to look solid, I'll add my review now.

I've also updated my test Docsify-This v2 build to use your updated Sidebar docsify.min.js for continued testing, and you can try it out at https://preview-v2.docsify-this.net/?basePath=https://raw.githubusercontent.com/hibbitts-design/docsify-this-multiple-page-open-publishing-site/main&homepage=home.md&sidebar=true&loadSidebar=_sidebar.md&docsify-core-theme=true#/

Copy link
Collaborator

@paulhibbitts paulhibbitts left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@sy-records sy-records merged commit 71ac41e into docsifyjs:develop Jun 14, 2025
8 checks passed
@sy-records sy-records deleted the fix/2556 branch June 14, 2025 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants