Integrate StickyTOC to chameleon as modification component

[xSavitar]

@note: This fixes deprecated call to Hook::run() as well.

[xSavitar]

NOTE: This is my first time doing chameleon components, so I'm learning as I go and would need someone to help me test and go review. Thanks!

[bawolff]

I tested and read through the code. Mostly looks good but I have some notes:

• [minor] I think maybe $wgStickyTOCFloat = "aside"; should be the default as the current default for that variable is a bit odd [I suppose if I am on the subject, it is a bit odd this is a config variable instead of something specified in a layout file, but perhaps that's out of scope of this change]
• stickytoc should be added to the layout.rng schema file.
• This seems to always enable sticky TOC if you are using the stickyhead layout (Or more specifically the sticky modification is set, so i guess clean and fixedhead as well). That makes sense when this was a separate extension - presumably you wouldn't enable the extension if you didn't want the toc to be sticky. However presumably many people using the chameleon skin might right now with the stickyhead layout might not want a sticky toc. So i think this shouldn't be included as part of the sticky modification only the stickytoc modification. Perhaps a new layout file could be added just for sticky toc.

[xSavitar]

Thanks for reviewing @bawolff.

I tested and read through the code. Mostly looks good but I have some notes:

• [minor] I think maybe $wgStickyTOCFloat = "aside"; should be the default as the current default for that variable is a bit odd [I suppose if I am on the subject, it is a bit odd this is a config variable instead of something specified in a layout file, but perhaps that's out of scope of this change]

For this one, I'm still trying to understand what "aside" means. Does it mean "left" or "right" aside? Maybe but I have no issue with setting it as the default instead of "left". Just the ambiguity in the word "aside" for me.

• stickytoc should be added to the layout.rng schema file.

Done!

• This seems to always enable sticky TOC if you are using the stickyhead layout (Or more specifically the sticky modification is set, so i guess clean and fixedhead as well). That makes sense when this was a separate extension - presumably you wouldn't enable the extension if you didn't want the toc to be sticky. However presumably many people using the chameleon skin might right now with the stickyhead layout might not want a sticky toc. So i think this shouldn't be included as part of the sticky modification only the stickytoc modification. Perhaps a new layout file could be added just for sticky toc.

Done! I've introduced a new stickytoc.xml file. And applied the chameleon modification for sticky-toc on the content. Let me know if that makes sense to you.

[bawolff]

     For this one, I'm still trying to understand what "aside" means. Does it mean "left" or "right" aside? Maybe but I have no issue with setting it as the default instead of "left". Just the ambiguity in the word "aside" for me.

aside means the TOC is separate from the text on the left side, where 'left' basically means it overlaps text after scrolling. I guess it is just personal preference and doesn't matter too much.

[bawolff]

Done! I've introduced a new stickytoc.xml file. And applied the chameleon modification for sticky-toc on the content. Let me know if that makes sense to you.

Yep that looks good. I think we should also not apply the stickytoc setting to things setting sticky but not stickytoc for backwards compatibility.

[bawolff]

It looks like there is also a typo on the script path for sticky.js

[bawolff]

I made a pull request for the two small remaining issues. https://github.com/xSavitar/chameleon/pull/1

[xSavitar]

I made a pull request for the two small remaining issues. xSavitar#1

Thank you @bawolff

[malberts]

In additional to my review above, I'm wondering if we shouldn't take a difference approach here. Instead of pulling the TOC next to the main content, can't we make it place the TOC where it is placed in the layout? So for example if I have a sidebar layout, then I might want the sticky TOC there, not just simply pulled next to the content.

For example, with this layout snippet:

<row>
    <cell class="border col-3">
        <modification type="Stickytoc"/>
    </cell>
    <cell class="col-9">
        <component type="MainContent"/>
    </cell>
</row>

I would've expected the TOC to go where I placed it in the layout (the bordered sidebar):

(This also illustrates the issue with this being a Modification that does not actually modify the parent Component.)

---

Or is this current implementation solving a specific use case which is not necessarily supposed to handle my use case example?

[krabina]

I agree that the TOC should go in the bordered sidebar.

[krabina]

@xSavitar do you think you will find time to update your PR?

[malberts]

@krabina We have since merged an alternative approach, although it is currently undocumented and needs more testing for custom layouts. On the latest master branch, you can add the Toc component where you want to move the original TOC (e.g. into a sidebar):

<component type="Toc"/>

I don't currently have a wiki to show, but here is an early video of it:

https://github.com/ProfessionalWiki/chameleon/assets/1428594/48bcf4f2-d7aa-4acf-9830-a3acab6190c2

[krabina]

Looking great! Can you share your layout.xml file?

[malberts]

@krabina Here's a basic example:

<?xml version="1.0" encoding="utf-8"?>
<structure xmlns="https://ProfessionalWiki.github.io/chameleon/schema/3.5/layout.rng">

    <component type="NavbarHorizontal" class="sticky-top">
        <component type="Logo" position="head"/>
        <component type="NavMenu" flatten="navigation"/>
        <component type="PageTools" class="flex-row" position="right" hideSelectedNameSpace="yes"/>
        <component type="SearchBar" class="order-first order-cmln-0" position="right" buttons="go"/>
        <component type="PersonalTools" position="right"/>
    </component>

    <grid mode="fluid">
        <row>
            <cell class="d-none d-md-block col-md-3 col-lg-2 border-right">
                <component type="Toc"/>
            </cell>
            <cell class="col-12 col-md-9 col-lg-10">
                <component type="SiteNotice"/>
                <component type="MainContent"/>
            </cell>
        </row>
    </grid>

    <grid class="mb-2 mt-4" mode="fluid">
        <row>
            <cell>
                <component type="NavbarHorizontal" collapsible="no" class="small mb-2" >
                    <component type="Toolbox" flatten="no" class="dropup"/>
                    <component type="LangLinks" flatten="no" class="dropup"/>
                </component>
            </cell>
        </row>

        <row>
            <cell>
                <component type="FooterInfo"/>
                <component type="FooterPlaces"/>
            </cell>
            <cell>
                <component type="FooterIcons" class="justify-content-end"/>
            </cell>
        </row>
    </grid>
</structure>

[krabina]

In my first test it looks great and is working. However, the "hide" "show" for the TOC is not working and in the browser console I see this:

Uncaught TypeError: $(...).scrollspy is not a function
    <anonymous> https://...i/skins/chameleon/resources/js/Components/toc.js?eb8ea:48
    jQuery 2
        mightThrow
        process

[krabina]

Another issue: on pages where there is no TOC at all, the space for the TOC should not be wasted.

[kghbln]

Another issue: on pages where there is no TOC at all, the space for the TOC should not be wasted.

What should be shown instead? I personally would not like to expand the content area to the sidebar.

[krabina]

Oh yes, exactly that. I have a wiki where the customer wants this because of very long content pages. There it is a great feature. But on many other pages, content is shown with different result formats (tables, etc.). There every inch of screen size is precious. I really only need the StickyTOC on the left side if there is a TOC. Since you can deliberatly chose this with TOC NOTOC (and even FORCETOC it would be a great feature to only use the space once a TOC should be there and give it back to the main content of there is no TOC

[krabina]

BTW: an option position="right" would also be great, since a blank space on the left is much more annoying than a blank space on the right. Tweeki does it like this: https://tweeki.kollabor.at/wiki/How-Tos

[malberts]

Another issue: on pages where there is no TOC at all, the space for the TOC should not be wasted.

The difficulty here is that the sidebar is defined in the layout, independent of the TOC. So you could also put other things in the sidebar, or even define the sidebar in a completely different way. Having the TOC then take control of a parent element/component is kind of out of scope for the TOC component. But we could perhaps add a class to the tag to indicate that a TOC is present/absent. And then the admin can add custom styling to hide the whole sidebar.

[krabina]

I observed an interesting effect: If you use the result format "datatables" on a page, it will somehow use the left sidebar as well. This is good, because datatables usually need as much width as they can get, but it is somehow unintended. So maybe this gives a hint on how this could be turned into a feature.

[DanielWTQ]

I've sent #405 to try and improve the styles of the component; I believe this can be closed

[malberts]

Closing this as superceded by work being done for https://github.com/ProfessionalWiki/chameleon/issues/390.