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

Table of contents in a <details> #36885

Closed
ghost opened this issue Jan 12, 2021 · 7 comments
Closed

Table of contents in a <details> #36885

ghost opened this issue Jan 12, 2021 · 7 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@ghost
Copy link

ghost commented Jan 12, 2021

Since table of contents on mobile is really tall in height i propose to put that in a <details>.

Screenshot_20210112-114658

@PoojaDurgad PoojaDurgad added the doc Issues and PRs related to the documentations. label Jan 12, 2021
@aduh95
Copy link
Contributor

aduh95 commented Jan 12, 2021

@mattiapontonio Would you like to send a PR? Let me know if you need help.

@aduh95
Copy link
Contributor

aduh95 commented Jan 12, 2021

You can set up the <details> here:

node/doc/template.html

Lines 58 to 61 in ec794f9

<div id="toc">
<h2>Table of Contents</h2>
__TOC__
</div>

And I'm from mobile. It's ok?

If you have the correct tools on your mobile, sure!

@ghost ghost mentioned this issue Jan 12, 2021
@ghost ghost changed the title Table of Contents in a <details> Table of contents in a <details> Jan 13, 2021
@aduh95 aduh95 closed this as completed in 2ba8728 Jan 14, 2021
@ghost
Copy link
Author

ghost commented Jan 14, 2021

Did It worked?

This Is not modified.

node/doc/template.html

Lines 58 to 61 in ec794f9

<div id="toc">
<h2>Table of Contents</h2>
__TOC__
</div>

Screenshot_20210114-165539.png

ruyadorno pushed a commit that referenced this issue Jan 22, 2021
PR-URL: #36896
Fixes: #36885
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@Trott
Copy link
Member

Trott commented Feb 23, 2021

Did It worked?

This Is not modified.

It's deployed now, but yes, it wasn't deployed at the time that you asked. It had to wait for a release to happen.

@ghost
Copy link
Author

ghost commented Feb 23, 2021

Did It worked?

This Is not modified.

It's deployed now, but yes, it wasn't deployed at the time that you asked. It had to wait for a release to happen.

Nothing seems to have happened, and PR has not merged either. But the code on master has changed. I really don't know, some kind of black magic.
I would have liked to see PR as merged.

@aduh95
Copy link
Contributor

aduh95 commented Feb 23, 2021

Apologies that your PR is not marked as merged, I'm not sure if it's a GitHub glitch or if I did something wrong when merging it, but rest assured your commit (2ba8728) has landed on master, and was included in the v15.7.0 release.
Now if you visit the documentation web pages, you'll see the ToC is now in a details tag: https://nodejs.org/api/esm.html

@ghost
Copy link
Author

ghost commented Feb 23, 2021

It works 👍 No need to apologize. Maybe it's due to the flow, I don't know. Perfect anyway. Thanks.

targos pushed a commit that referenced this issue May 1, 2021
PR-URL: #36896
Fixes: #36885
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants