backdrop-contrib / tocbot

Builds a table of contents (TOC) from content headings.
GNU General Public License v2.0
2 stars 2 forks source link

Improve configuration form for advanced settings #35

Open olafgrabienski opened 4 years ago

olafgrabienski commented 4 years ago

Follow-up for #27.

In #27 @stpaultim expected that by removing is-collapsible from the configuration form the TOC would no longer be collapsible and that by removing is-collapsed it would remain collapsible, but not by default. If he removed both, he expected the TOC would neither be collapsed or collapsible.

AFAIK the mentioned classes are not meant to be removed but at best to be changed if people want to use other class names for some reason. (In that case people also need to adapt the CSS to get the out-of-the-box behavior.) To improve the issue, it would be good to

Additional ideas:

benravetta commented 3 years ago

So, does this mean that I need to add my own CSS in order for the ToC to be collapsible?

olafgrabienski commented 3 years ago

Hey Ben, you don't have to add custom CSS to get the collapsible behavior. That said, speaking of the TOC being collapsible, how I did in the issue description, isn't very accurate. The collapsible behavior applies to lists within the TOC that hold sub items, like the <ol> element in the (simplified) example below:

<li class="toc-list-item">
  <a href="#my-item" >My Item</a>
  <ol class="toc-list is-collapsible">
    <li><a href=...>Sub item 1</a></li>
    <li><a href=...>Sub item 2</a></li>
    <li><a href=...>Sub item 3</a></li>
  </ol>
</li>

(Note: the example is from the TOC on https://tscanlin.github.io/tocbot)

The expected behavior is: TOC sub items are hidden (the holding <ol> is collapsed) as long as the respective page sections are out of the browser viewport. The sub items get visible when you jump or scroll to the respective sections.

benravetta commented 3 years ago

Thanks for getting back to me. Sorry, I didn’t make myself clear. What you are describing is exactly what I want. But for some reason, all of the subsections are already expanded. I would like to have them collapsed and expand on scroll. Is there something I need to add to the confit init to make sure this happens?

Best Wishes,

Ben Ravetta | Company Director brightwork.uk Visit the forum community. Need support? Open a ticket with us here. This message is intended exclusively for the individual(s) to whom it is addressed and may contain information that is privileged, or confidential. If you are not the addressee, you must not read, use or disclose the contents of this e-mail. If you receive this e-mail in error, please advise us immediately and delete the e-mail. Brightwork have taken every reasonable precaution to ensure that any attachment to this e-mail has been swept for viruses. However, Brightwork cannot accept liability for any damage sustained as a result of software viruses and would advise that you carry out your own virus checks before opening any attachment. On 13 May 2021, 11:04 +0100, Olaf Grabienski @.***>, wrote:

Hey Ben, you don't have to add custom CSS to get the collapsible behavior. That said, speaking of "the TOC" being collapsible, how I did in the issue description, isn't very accurate. The collapsible behavior applies to lists within the TOC that hold sub items, like the

    element in the (simplified) example below:

  1. My Item
    1. Sub item 1
    2. Sub item 2
    3. Sub item 3
  2. (Note: the example is from the TOC on https://tscanlin.github.io/tocbot) The expected behavior is: TOC sub items are hidden (the holding

      is collapsed) as long as the respective page sections are out of the browser viewport. The sub items get visible when you jump or scroll to the respective sections. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

olafgrabienski commented 3 years ago

for some reason, all of the subsections are already expanded. I would like to have them collapsed and expand on scroll.

Hm, they shouldn't be already expanded. There is one exception, though: subsections of the top of the page are already visible in the TOC when you visit the page. I guess that's because you are already "there" when visiting the page. As an example, see the "Get started" sub-items of the library demo website: https://tscanlin.github.io/tocbot

However, subsections from other page sections should not be expanded when you visit the page. I've just verified it on one of my test sites. (As soon I find time, I can set up a vanilla demo site with out of the box settings.)

olafgrabienski commented 3 years ago

Okay, I've set up a temporary demo website using https://backdropcms.org/demo. The collapsible behavior works for me, without changing any Tocbot settings. Steps to reproduce:

Below is the link to the demo page. Should be online for about 18 hours from now. If you don't have a chance to have a look at it, you could create your own and follow the steps mentioned above.

https://latest-kj0xleixgpm8di9o3f6cvdrhog4wsxkt.tugboat.qa/test-page-tocbot

benravetta commented 3 years ago

Okay, I've set up a temporary demo website using https://backdropcms.org/demo. The collapsible behavior works for me, without changing any Tocbot settings. Steps to reproduce:

  • Set up the demo site.
  • Install Tocbot via the project browser.
  • Add the Tocbot block to the sidebar of the Default layout.
  • Create a page with a lot of h3 headings (for sections) and h4 headings (subsections).

Below is the link to the demo page. Should be online for about 18 hours from now. If you don't have a chance to have a look at it, you could create your own and follow the steps mentioned above.

https://latest-kj0xleixgpm8di9o3f6cvdrhog4wsxkt.tugboat.qa/test-page-tocbot

Wow, I really appreciate that. Yes, looks to be working great. I'm really not sure what is causing my own to be permanently expanded and non-collapsible. I wonder if there's some rogue JS somewhere. Here's the code I have currently.

   ```<div>
      <div class="js-toc"></div>
    </div>

  <script src="https://cdnjs.cloudflare.com/ajax/libs/tocbot/4.11.1/tocbot.min.js"></script>
    <script>
            tocbot.init({
              tocSelector: '.js-toc',
              contentSelector: '.js-toc-content',
              headingSelector: 'h1, h2, h3, h4',
              hasInnerContainers: true,
              activeListItemClass: 'is-active-li',
              collapseDepth: 0,
              scrollSmooth: true,
              scrollSmoothDuration: 420,
              headingsOffset: 100,
              scrollSmoothOffset: -100,
            });
    </script>```

Then obviously the selector class .js-toc-content is included in the div where the content is.

olafgrabienski commented 3 years ago

Sorry, I have very little JS skills :-/

Not sure if it matters but I'm wondering why you're loading the external Tocbot library. It's actually bundled with the Backdrop module, see https://github.com/backdrop-contrib/tocbot/issues/11#issuecomment-560454766.

benravetta commented 3 years ago

Sorry, I have very little JS skills :-/

Not sure if it matters but I'm wondering why you're loading the external Tocbot library. It's actually bundled with the Backdrop module, see #11 (comment).

Hi, thanks for all your help man. I am unsure how I even got here, to be honest. I thought I was on the Tocbot Github, but just realised it's this Backdrop CMS thing. That makes so much more sense now. I loaded up your demo, I was like, "What's Backdrop?" I'm running Tocbot on Ghost CMS 🤣 I managed to fix my issues anyway, but loading up the default tocbot CSS file from the CDN which I wasn't including. That allowed the collapse to happen.

Thanks again!

herbdool commented 3 years ago

I suggest putting the collapsible depth field first and saying that if set to 6 that it effectively turns off collapsible.

Maybe some fieldsets would help too. Help people understand which settings are related.

olafgrabienski commented 3 years ago

Thanks for your suggestions @herbdool! I'll have a look when I find time.

olafgrabienski commented 3 years ago

I suggest putting the collapsible depth field first (...)

What do you mean by "first"? First JavaScript setting at all, or first Collapse related setting?

olafgrabienski commented 3 years ago

(...) and saying that if set to 6 that it effectively turns off collapsible.

For reference, this is the current situation:

collapseDepth (number) [ input for number value ] How many heading levels should not be collapsed. Number 6 will show everything since there are not more than 6 heading levels. Number 0 will collapse all sections, and they will open and close as you scroll to them. Default: 0

@herbdool Instead of "Number 6 will show everything ...", shall we just say "Number 6 will turn off collapsible behavior ..."? Or would you prefer to combine the phrases? ("Number 6 will turn off collapsible behavior and show everything ...")

Actually, other parts of the current description are also a bit hard to understand (e.g. how many heading levels "should not be collapsed"). I'm open for suggestions!

herbdool commented 3 years ago

What do you mean by "first"? First JavaScript setting at all, or first Collapse related setting?

I think I meant the latter: first collapse related setting.

herbdool commented 3 years ago

How many heading levels should not be collapsed. Number 6 will show everything since there are not more than 6 heading levels. Number 0 will collapse all sections, and they will open and close as you scroll to them.

This part sounds pretty awkward in English. Maybe something like:

The level where headings are collapsed, between 0 and 6. Enter 6 to show all headings. Enter 0 to collapse all headings, except for the section in focus while scrolling.

olafgrabienski commented 3 years ago

Maybe something like:

The level where headings are collapsed, between 0 and 6. Enter 6 to show all headings. Enter 0 to collapse all headings, except for the section in focus while scrolling.

Thanks for the suggestion, sounds already better! I've just had a look at the behavior and realized that the former text was misleading in mentioning the number of levels ("how many") instead of the level itself. We could try to make even more clear that the numbers are referring to the HTML heading levels. Text areas are usually configured to start with H3 in Backdrop. In this case, when you put a 2 in the field, nothing happens because it's related to H2.

olafgrabienski commented 3 years ago

@herbdool I'm on the verge of a vacation and can't be active here for the next weeks. In case you need the issue to be fixed before, feel free to become a co-maintainer of Tocbot. Interested?

herbdool commented 3 years ago

I'm not in a hurry to get this fixed. I think I've taken on enough module maintaining for now. Thanks for your help on this!