doc: Improve navigability of API docs

[ShogunPanda]

This PR reworks the top header in API docs adding the following features:

• The TOC of the current page has been added as dropdown menu.
• The API index has been added as dropdown menu.
• The header is now sticky on screens taller than 1000px

This is also a preparatory work for a future PR which will improve the usability of the docs by making the header smaller when pinned and the dropdown menu toggled by clicks when JS is enabled.

Hope this helps!

[Trott]

@nodejs/website

[Trott]

Non-blocking questions/suggestions:

• I wonder if it might be better if only the nav bar part of the header were sticky rather than the whole header. I personally don't feel like it's a great use of my screen real estate to have the big header text sitting there the whole time. I want to read the doc contents, not "Node.js v17.3.0 documentation" in a large typeface. (Another option might be for that text to shrink when the user scrolls.)
• Not new in this PR but exacerbated: I worry about usability and accessibility implications of the hover menus. Might it be better if they expanded on click rather than on hover?
• Not new in this issue but arguably exacerbated: I also worry about the possibility of confusing UI with the green triangle. In the main text, at least on Chrome, a triangle is used next to the Table of Contents header to mean "click here to toggle expand/hide for the Table of Contents". But in the menu, it's being used for "hover here to see menu selections". Additionally, in the former case, the triangle rotates depending on whether clicking it will expand or hide the contents. The triangles added here (and the one that already exists for the version picker) are always static.
• The API index dropdown added here duplicates the contents of the left sidebar. But the sidebar isn't visible in narrow viewports. Seems like a lot of duplication in wide viewports. Should we get rid of the sidebar?

[Trott]

Oh, this might be a blocking comment: This is not good on narrow viewports (e.g. a smartphone). The sticky header takes up most of the screen. Maybe use a CSS media query to decide when to unstick the header or something like that?

[ShogunPanda]

Non-blocking questions/suggestions:

• I wonder if it might be better if only the nav bar part of the header were sticky rather than the whole header. I personally don't feel like it's a great use of my screen real estate to have the big header text sitting there the whole time. I want to read the doc contents, not "Node.js v17.3.0 documentation" in a large typeface. (Another option might be for that text to shrink when the user scrolls.)

You are right, it's a waste of space. I'll see to rework it to make it much smaller.

• Not new in this PR but exacerbated: I worry about usability and accessibility implications of the hover menus. Might it be better if they expanded on click rather than on hover?

I agree on this. I was also worrying about this when I worked on it but I conservatively chose to keep the existing approach.
I'll now instead rework it to rely on clicks/taps.

• Not new in this issue but arguably exacerbated: I also worry about the possibility of confusing UI with the green triangle. In the main text, at least on Chrome, a triangle is used next to the Table of Contents header to mean "click here to toggle expand/hide for the Table of Contents". But in the menu, it's being used for "hover here to see menu selections". Additionally, in the former case, the triangle rotates depending on whether clicking it will expand or hide the contents. The triangles added here (and the one that already exists for the version picker) are always static.

Given my reply above, I'll also take care of this. Click will switch triangle like in the default browser behavoir.

• The API index dropdown added here duplicates the contents of the left sidebar. But the sidebar isn't visible in narrow viewports. Seems like a lot of duplication in wide viewports. Should we get rid of the sidebar?

I would go for the first instead. I'll make sure that API index dropdown is only visible when the sidebar is not visible. So we keep the best.

Oh, this might be a blocking comment: This is not good on narrow viewports (e.g. a smartphone). The sticky header takes up most of the screen. Maybe use a CSS media query to decide when to unstick the header or something like that?

Noted. I'll take care of this as well.

[ShogunPanda]

Hello! I've update the PR with the following modifications:

1. Pickers are now based on clicks/tap rather than hovering. I've updated arrows semantic to match <detail> tag behavior.
2. The sticky header is much smaller and it is not visible at all for narrow screens (where it would been hard to manage)
3. The "Index" picker in the header is not visible if the sidebar is visible.

Let me know if it is any better now.

[Trott]

@nodejs/website

[Trott]

@ShogunPanda Looks like it's moving in the right direction. When I'm on the index.html page, the Table Of Contents drop-down menu doesn't work. It works on other pages though. Are you seeing that same problem or might that be something with my local configuration?

[ShogunPanda]

@Trott No, it was a bug I forgot to fix. It is fixed now.

[Trott]

Getting very close. The "Edit on GitHub" link wraps in a weird/confusing way for me at some browser widths. Is this something that can be fixed easily?

Maybe one solution is to change "View another version" to "Other versions"?

[ShogunPanda]

@Trott I've changed to "Other Version" and also added a CSS rule to avoid text wrapping.

[Trott]

Not a blocker in my opinion but I'm still not sure about this wrapping at somewhat narrow widths:

I now see that the current site has some wrapping-at-narrow-viewport going on like that. So definitely not a blocker.

[Trott]

Looks good to me with the change that aduh95 requested. (If it reports a lint error, that's a problem and we should find out why!) I'm sure this can still be improved (like everything), but I think we can go with it and iterate on stuff.

[aduh95]

Including a non-markdown link because the linter complains is not the correct thing to do imho. We should either:

• change the linter so it accepts a self-reference for index.md,
• add a comment asking the linter to ignore this one,
• don't include the link in the markdown file but inject it in the HTML at build time.

The latter seems like the best options of all three, but let me know what you think.

[aduh95]

Thanks for addressing this issue :)

There's some glitch to fix when JavaScript is disabled:

In general, this does improve the navigability of the docs when JS is enabled, but dramatically degrade it when it's disabled. Can you have a look at that?

[ShogunPanda]

@aduh95 @Trott I've solved the markdown issue in another way, dynamically injecting index.html in the header.

@Trott About your wrapping question: I've moved the last three options in a submenu and changed some breakpoints. I'm not able to reproduce the wrapping problem on my screen anymore.

@aduh95 With a little more JS hacking, I was able to detect whether the browser has JS enabled or not. If JS is disabled, the header is not sticky and the menu works with the previous hovering logic.

Note that since the JS inside the template was growing, I moved it outside as a separate file.

[aduh95]

I find the hovering version more usable, especially on mobile, and I'm not a fan of duplicating the expanding logic. The JS and CSS are indeed growing, I'd prefer if we were shipping way less.

Can we keep the stickiness when JS is disabled, but add a background color to it to avoid the glitch?

[ShogunPanda]

Hovering on mobile is not possible, since they are touch devices - See the second point in #41404 (comment).

I'll keep the header sticky when possible (so not in small screens where it might take the entire vertical spaces)

[aduh95]

Hovering on mobile is not possible, since they are touch devices

Not sure what you mean, I think it works quite well – I'm using iOS 15.2, but I'd be very surprised if it wasn't also available on Android.

I'll keep the header sticky when possible (so not in small screens where it might take the entire vertical spaces)

Yep good idea, I suppose a media query to ensure the vertical size is enough should do the trick.

[ShogunPanda]

Not sure what you mean, I think it works quite well – I'm using iOS 15.2, but I'd be very surprised if it wasn't also available on Android

Do you mean that the menu shows up when you tap?
If that's the case, that's a heuristic emulation done by Safari. Basically the :hover behaviour is emulated via a tap. It's non standard and might go away at any time.

Also it requires a tap on another clickable element to "hover-out". I don't like the usability of this at all, to be honest.

[aduh95]

OK thanks for clarifying that, the JS changes make more sense now.

Would you agree to split this PR in two? I'm thinking we could make one part that focuses on the CSS changes, and a second one later for the JS changes. I find it hard to review this PR because I feel overwhelmed by the size of it, it'd be easier for me to do it in smaller chunks.

[ShogunPanda]

I would love to, but CSS changes are mostly driven by JS.
Is the split absolutely be necessary?

[ShogunPanda]

@aduh95 Header is now sticky on 1000px+ screens

[ShogunPanda]

@aduh95 I made the header sticky. Are we ready to land this?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42721/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42725/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42727/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/42749/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/41404
✔  Done loading data for nodejs/node/pull/41404
----------------------------------- PR info ------------------------------------
Title      doc: Improve navigability of API docs (#41404)
Author     Paolo Insogna  (@ShogunPanda)
Branch     ShogunPanda:improve-api-docs-layout -> nodejs:master
Labels     doc, tools, author ready, commit-queue-squash
Commits    3
 - doc: improve navigability of API docs
 - doc: make the header sticky for bigger screens
 - doc: Make header sticky by default.
Committers 2
 - Shogun 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/41404
Reviewed-By: Rich Trott 
Reviewed-By: Antoine du Hamel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41404
Reviewed-By: Rich Trott 
Reviewed-By: Antoine du Hamel 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - doc: Make header sticky by default.
   ℹ  This PR was created on Wed, 05 Jan 2022 13:39:44 GMT
   ✔  Approvals: 2
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/41404#pullrequestreview-866837564
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/41404#pullrequestreview-889681286
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-02-23T18:21:34Z: https://ci.nodejs.org/job/node-test-pull-request/42749/
- Querying data for job/node-test-pull-request/42749/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1891433886

[aduh95]

Landed in 9be1dcf