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

Configurable ToC Heading Levels #5101

Merged
merged 6 commits into from
Apr 4, 2022
Merged

Configurable ToC Heading Levels #5101

merged 6 commits into from
Apr 4, 2022

Conversation

TimoKruth
Copy link

@TimoKruth TimoKruth commented Mar 16, 2022

@NGPixel as you wrote

I would consider accepting this PR for 2.x as the next version still require more time. I would however want some UI changes as to not rely on 10 radio boxes, which is a bit too busy. A range slider would be more appropriate, example in 3.0:

image

Also, the behavior should be more in line with how 3.x will work, with a min and max value:

  • min: The minimum heading that will be shown. A value of 2 would ignore h1 nodes but still show their children (if any).
  • max: The maximum heading that will be expanded. Lower headers are still accessible but collapsed by default.

These changes shouldn't require much work but it would make the transition to 3.x much easier.

I added a range slider as config choice for ToC heading selection. Please review and give your opinion.
Yeah and I have no idea how to put this in the original PR. Sorry for that.

Maybe we should make the collapse Level a dropdown menu?

The look of my work:
Page option:
Bildschirmfoto 2022-03-17 um 07 11 39

Admin option
Bildschirmfoto 2022-03-17 um 07 15 33

regevbr and others added 4 commits July 13, 2020 01:59
test

Revert "test"

This reverts commit f4959c777a013d2c7966e6cc5b67ee29a067e417.

Revert "First parts of reworking ToC Slider"

This reverts commit 08cb4bd269a628f71f935e812b3980322a9d7303.

Range Slider instead of radio button

range slider in admin-theme
@TimoKruth
Copy link
Author

@NGPixel Any Feedback on this?

@NGPixel
Copy link
Member

NGPixel commented Mar 23, 2022

Thanks for this PR!

Some points to address:

  • Global shouldn't be part of the slider. It should be a toggle above or the left of the slider that enables / disables it.

  • What is the Collapse Heading Level for? It should be removed completely as it's already be handled by the slider above it.

Slider Range Values:

  • Left value = Level to start displaying headers from.
  • Right value = The lowest level to show headers expanded. (a value of h3 means that h4 and lower are collapsed by default)

Here's a quick mockup of what it should look like:

When Use Site Defaults is enabled (default):

image

When Use Site Defaults is disabled:

image

Collapse level is now controlled by upper limit of toc level range slider
everything beneath is collapsed
@TimoKruth
Copy link
Author

TimoKruth commented Mar 24, 2022

@NGPixel
Do you mean like the following?

Setting in Editor:
Bildschirmfoto 2022-03-24 um 20 40 29

View of ToC and Site:
Bildschirmfoto 2022-03-24 um 20 40 17

I will add the Use Site Default Toggle.

Use Site Default Switch now changes between usage of page specific toc levels and site specific toc levels per site.
Removed Global selection from range slider of toc level therefor
@TimoKruth
Copy link
Author

TimoKruth commented Mar 25, 2022

@NGPixel Toggle Switch is now implemented and working as I would expect it. Please feel free to check that for yourself.
Following the relevant Screenshots:

Admin Theme Setting:
Bildschirmfoto 2022-03-25 um 12 34 28

Page Editor (once using site default once not):
Bildschirmfoto 2022-03-25 um 12 32 38

Bildschirmfoto 2022-03-25 um 12 32 48

Page using Site Default:
Bildschirmfoto 2022-03-25 um 12 34 12

Page using Page Setting:
Bildschirmfoto 2022-03-25 um 12 33 39

@NGPixel
Copy link
Member

NGPixel commented Mar 26, 2022

In your screenshots, everything looks great, with the exception of the collapse defaults. If the right end of the slider is set to H2, then H2 should be visible but any children (H3, H4, H5 and H6) should be collapsed and not visible by default (they can still be expanded by the user though).

@TimoKruth
Copy link
Author

Then everything is as you expect it. The upper one is expanded by hand to show how it works. The arrows are blue instead of black after clicking on it and show upwards instead of downwards. The lower part of the ToC is the normal not clicked version. That's why I added two of them, to be able to showcase both at the same time.
Hope this clarifies things. If not let me know :)

@NGPixel NGPixel added under review Acknowledged, awaiting further review and removed needs-work labels Mar 29, 2022
@NGPixel NGPixel changed the title Work for #2189 fixing #1216 Configurable ToC Heading Levels Apr 3, 2022
@NGPixel NGPixel changed the base branch from main to feat-toc April 4, 2022 03:21
@NGPixel NGPixel merged commit c49938a into requarks:feat-toc Apr 4, 2022
biocrossroads pushed a commit to biocrossroads/biocrossroads-wiki that referenced this pull request Apr 24, 2022
NGPixel pushed a commit that referenced this pull request Jun 5, 2022
@NGPixel NGPixel mentioned this pull request Jun 6, 2022
2 tasks
@FlorinBuffet
Copy link

@NGPixel Why is this only in the feat-toc branch? Because nowadays this one is much behind the main branch. Could this also be pushed to the main branch?

@cyr0nk0r
Copy link

@FlorinBuffet, agreed. Why was this merged into a 'stale' branch?
This request has been floating around since 2020 in various forms.
Is there any way to get this into the main branch?

@NGPixel
Copy link
Member

NGPixel commented Oct 17, 2023

@cyr0nk0r Because it's not ready for production use. It has many display bugs that need to be resolved first.

@cyr0nk0r
Copy link

perfect is the enemy of good. I think the people that want this (myself included) would rather deal with a few display bugs and have something for the last 18 months than nothing at all. It's going to get revamped and be fixed anyway in 3.0.
So why not?

@TimoKruth
Copy link
Author

Hej @NGPixel if you have a short mention of the display bugs that need to be resolved I could try that and make a new MR if that helps anything

@pez252
Copy link

pez252 commented Jul 31, 2024

Hi @NGPixel is there any update on this? It looks like @TimoKruth is interested in helping to resolve any display bugs. This feature would be quite helpful for my use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement under review Acknowledged, awaiting further review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants