-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
feat(v2): redesign mobile UX: inline TOC + doc sidebar in main menu #4273
Conversation
This comment has been minimized.
This comment has been minimized.
✔️ [V2] 🔨 Explore the source changes: 3dd78dc 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60e85f99b4bcbe00087d3d6e 😎 Browse the preview: https://deploy-preview-4273--docusaurus-2.netlify.app |
59fc2c3
to
d14405b
Compare
d14405b
to
fa06ace
Compare
Size Change: -30.9 kB (-3%) Total Size: 965 kB
ℹ️ View Unchanged
|
This comment has been minimized.
This comment has been minimized.
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4273--docusaurus-2.netlify.app/ |
The last commit looks like it fixed the "ToC popup doesn't hide when link tapped" issue, but for some reason the popup is still empty of any link content in Chrome for Android. |
That looks pretty cool, thanks for working on this! Some design feedbacks:
From a technical perspective, will have to take a deeper look. My initial idea was to avoid using the global data API for this feature because the data is "global", ie it is available on all the pages of the site, and having a long sidebar means it impacts the whole site performance (including the homepage, and we have Docusaurus sites with huge sidebars like this one: https://xsoar.pan.dev/docs/reference/index) What I would try is to replace the global data API with a "portal" / "slot-fill" library:
The problem is that afaik there's no up-to-date maintained lib that supports SSR:
Maybe we don't need to support SSR for the mobile sidebar (as we don't display the sidebar before the user click on the hamburger menu, there wouldn't be FOUC), but at the same time it may be important for SEO to render that sidebar in the markup as crawlers are mobile-first? (don't know if this matters because anyway it's hidden before you open the mobile menu) I would try to use react-teleport instead of the global data API and see if this can solve the use-case in a more efficient way |
@markerikson could you please give more details on this (source, screenshots)? I tested in this browser and didn't notice this issue. @slorber Why is using global data bad in this case? It seems to me that this approach works well, especially since we can remove the |
This comment has been minimized.
This comment has been minimized.
Yes that's what I reported. @lex111 go ahead with global data for now, as sidebars data is not huge and I'll try to see if I can refactor/optimize that later. It's better to have this feature asap even if it's not the ideal implementation IMHO, and we don't document docs global data APIs on purpose currently so we can do breaking changes on it later. |
FYI for sites with a very large sidebar, rendering the sidebar on the server for each doc page is expensive (cf #4753, just the sidebar increase a site's build time by 12 minutes!) We should test this new mobile UX against such a large site and see the potential impacts on build time and runtime perf. |
This comment has been minimized.
This comment has been minimized.
Sorry for such a long wait! I decided to completely rethink previous UI design, and the obtained result seems to me to be the best user experience: TOC of page is available as a collapsible panel by pressing button in front of the document heading, and the sidebar contents are displayed as a separate app-like menu. Hope you enjoy this option. Traditionally, there is internal implementation needs refactoring, and the styles do not fully support dark mode. This will all be fixed after we come to a final decision. |
@lex111 this looks great! I really like the inline collapsible TOC - great solution for mobile! |
Hmm. I appreciate the effort that went into this, but I'm not convinced it's the right approach. It looks to me like any time I want to access the in-page TOC, I'd first have to scroll all the way back up to the top of the page to expand that dropdown. That could easily get annoying on long docs page. This was actually a great thing about having the floating button. It's accessible no matter how far down you've scrolled on the current page. Personally I'd still really like to see the approach I first suggested: keep the floating button, and use that to trigger showing the in-page TOC. |
I was thinking about that. But later we will have "back to top" button that will make it easier to go to TOC collapsible panel. So given that, I don't want to clutter the interface with two floating buttons. Make sense for you? |
Heh. Having any way to access the page TOC on mobile is an improvement over what we've got now :) To me, having that floating button bring up a TOC overlay makes the most sense design-wise, but since you're the one implementing this and I'm over here in the peanut gallery, I can't complain too much - I'll take whatever actually gets implemented at this point. |
Thank you for understanding ;) In fact, I researched a lot of docs sites to come up with this new solution that I feel is most appropriate for needs of all users. As always, my concern is for users to focus on the main thing on site -- its content. And the current UI is the best way to do that (imo). |
My 2 cents - although I can understand and respect the idea of the floating button allowing access to the TOC no matter where one is currently scrolled on the page, I have to say in my experience I find those floating buttons mostly annoying / blocking the view of the content and rarely use them. I think a "back to the top" and then having the TOC there, like @lex111 has in their latest update is a perfectly fine compromise. |
@slorber I think the redux website gives double options (same items in navbar and sidebar). Once I pulled the sidebar, I can navigate from there. I don't really need to go Back to main menu in order to get the same sidebar. But this certainly makes sense if one has super long sidebar that would save one from scrolling up/down. You select any item in the main menu, you get the same sidebar but at different vertical scroll positions. I think if an item has sidebar, the expanded sidebar should stay there to let me choose. If different items on the main menu has separate sidebars, it feels abrupt otherwise. If one touches outside the expanded sidebar to hide it after selecting on the main menu, the behaviour should be as it is now. Just my thoughts. Thank you. |
Think about the user being on the homepage and clicking a navbar item. It's not even about going back, but what to expect on a navbar item click. With your UX, to access the Redux tutorial, it requires to press first on the sidebar tutorial element, and then to press on the sidebar overlay to close the sidebar (which is showing the docs sidebar, focusing on the tutorial element). Do we really want the user to press 2 times in this case? @markerikson any thoughts?
Technically we currently can't know if a navbar item belongs to a doc inside a sidebar, or an "orphan doc" without sidebar. This is because of the modular nature of Docusaurus: not all the data are available everywhere and we try to keep the "global data" (available on the homepage, blog etc...) very small because it impacts the perfs of your whole site. Having a new navbar item of type We could also have a boolean on navbar items to tell if the mobile sidebar should be closed on click or not. I already planned to implement the The question is: would this new sidebar item type solve your problem? Would you be able to replace your |
# Conflicts: # .github/workflows/v2-build-size-report.yml # packages/docusaurus-theme-classic/src/theme/DocSidebar/index.tsx
Yes, a new |
Thanks @pranabdas , actually tried to see how to make this UX work but it's not easy/possible in current state because our layout/navbar unmount/remount after each navigation and lose its opened state. This requires more complex refactorings to make this UX possible, so we'll figure this out in another PR later |
Let's merge this! 🎊 Please test this on your sites with https://docusaurus.io/community/canary |
@slorber thank you so much! i found a weird inconsistency when opening/closing menus - sometimes it would go to main menu, sometimes it would go to the TOC menu, and it doesnt seem very predictable: https://www.loom.com/share/d1ede4c5c04f41e9bd92785dcdac0e89 i havent figured out the cause yet, just reporting. i also dont think it should be a blocker since its already an improvement |
Thanks for the feedback 😜 and great unusual video bug report.
This looks like a valid bug.
While on a doc with sidebar, you should always end-up opening the mobile
sidebar on the TOC by default, consistently.
Will fix that soon, thanks for reporting.
Le ven. 9 juil. 2021 à 19:59, swyx ***@***.***> a écrit :
… @slorber <https://github.com/slorber> thank you so much! i found a weird
inconsistency when opening/closing menus - sometimes it would go to main
menu, sometimes it would go to the TOC menu, and it doesnt seem very
predictable:
https://www.loom.com/share/d1ede4c5c04f41e9bd92785dcdac0e89
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFW6PRV3F5RKVJENZKAHT3TW42G5ANCNFSM4YCLN27Q>
.
|
I don't see any major issues with this version, and mobile usability has improved significantly [1], so let's ship it. [1] facebook/docusaurus#4273
@sw-yx I was wondering why the mobile collapsible TOC does not show up here: Do you have a PR / preview so that I can inspect it more easily? I can also open a PR to the temporal doc if you want, and we try to get everything working properly here. |
We're on canary because of [1]. Upgrade for [2] and [3]. [1] facebook/docusaurus#4273 [2] facebook/docusaurus#5157 [3] facebook/docusaurus#5159
ah sorry just saw this! no i didnt have a separate PR opened, i just bumped versions to canary in local dev. thanks for handling it anyway!!! |
We're on canary because of [1]. Upgrade for [2] and [3]. [1] facebook/docusaurus#4273 [2] facebook/docusaurus#5157 [3] facebook/docusaurus#5159 Change-Id: Ic11b4480368fde47a82bd9613513972c51ffd8eb
Note from @slorber : this PR has been marked as a breaking change to draw attention that:
Test on mobile the new UX: https://deploy-preview-4273--docusaurus-2.netlify.app/docs/
Motivation
Attempt to resolve #2220.
Closes #4927
Closes #4700
Dirty/draft and apparently improper implementation, very quickly written to demonstrate new navigation UI (TOC) on mobiles (upon request from @markerikson).
The main idea is to render the doc sidebar content on mobiles in the navbar sidebar and also add a new sticky button to display the local l (by document) TOC. This gives us place for the back to top button.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)