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

Add TOC in sidebar for PSRs & PERs #321

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mathroc
Copy link

@mathroc mathroc commented Jul 16, 2023

This merge requests add a TOC in the sidebar for all PSRs & PERs (see #320)

The TOC is generated from the Markdown content, by taking the first 2 levels of titles

examples:

image

image

image

@mathroc mathroc marked this pull request as draft July 16, 2023 00:39
@Crell
Copy link
Contributor

Crell commented Jul 16, 2023

The TOC blocks need some styling yet, and should probably be indented by their sub-level, too. (I'm not sure if that's a template question or CSS question.) I'm in favor of the concept, though.

@mathroc
Copy link
Author

mathroc commented Jul 16, 2023

The TOC blocks need some styling yet, and should probably be indented by their sub-level, too. (I'm not sure if that's a template question or CSS question.) I'm in favor of the concept, though.

yes, as written in the description, the styling is missing (I'll give it a go later but if anyone is interested, I'd be more than happy to get some help here 😅) The template already put sub-level into child lists so the indentation should be no problem. I suppose there's a CSS reset somewhere that removed the default margins/padding so it appears flat

@Jean85
Copy link
Member

Jean85 commented Jul 17, 2023

Styling should also keep in mind mobile... And that's a bit hard 😓 any frontender can lend a hand here?

@mathroc
Copy link
Author

mathroc commented Jul 22, 2023

Hi y'all! I gave a try at styling this and made another change: I noticed the TOC would also be generated for other documents related to the PSR/PER listed in the "Additional Info" block, so I put the TOC in there just below its link.

I like it better, but let me know what you think

I also tried to make the Addition Info block sticky. But it's not working well if the block height is greater than the screen so I discarded it for now

(will update the screenshot in the description soon done)

@mathroc mathroc marked this pull request as ready for review July 22, 2023 09:36
@Crell
Copy link
Contributor

Crell commented Aug 18, 2023

Much improved. It would be useful to "fix" the TOC block when it's in the sidebar, though, so it doesn't scroll off the screen. (Fixing it exactly in place is trivial in CSS. Fixing it to scroll to the top of the page and then be fixed is better, but I'm not sure off hand if that requires any JS these days. I've lost track.)

@vdelau vdelau requested a review from a team August 18, 2023 15:12
@mathroc
Copy link
Author

mathroc commented Aug 18, 2023

@Crell It's possible to make it either fixed or sticky when we start scrolling with CSS only (eg: position: sticky;), but if the TOC block height is greater than the viewport height you can't access the bottom part :/

@mathroc
Copy link
Author

mathroc commented Sep 10, 2024

I tried again to make the sticky part works, but it's tricky when the TOC takes more than 100% of the screen height. It means it would have 2 scrollbars (at least once it start getting "stuck") which is not a great UX..

I think it's good to have even without it being sticky though. I'll ask a colleague who's better than me at CSS & UX when he's back from vacation if he has an idea of how to do that better but I would prefer doing so in a follow up PR if this one is good enough on its own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants