add ToC as in kiwix pwa

[Greeshmanth1909]

resolves #1212
Most of the code was taken from the actual pwa implementation. Variations from the original implementation include:

• The setupTableOfContents function would be called once the ToC drop down is clicked. The drop down only appears if an article is loaded in the iframe hence, the table of contents would be constructed once the user actually clicks the drop down.
• params.relativeFontSize was set to 100

This is how it looks:

[Jaifroid]

Thank you very much. I was away, so didn't have a chance to look at it till now. I'll take a look asap.

[Jaifroid]

I've had a quick look (I haven't reviewed the code so far). It's functional, which is great! I noticed a few cosmetic issues. The first is that the ToC button isn't vertically aligned in the same way as the other buttons on the bottom navbar. The second is that the ToC list opens attached to the bottom right of the screen instead of being attached to the button itself like in the PWA. And finally, there is no margin / padding around the text in the ToC list, which makes it difficult to distinguish from the article text. See screenshot. Could you kindly take a look at these issues?

[Greeshmanth1909]

Yes, will get this done by the end of this day.

[Greeshmanth1909]

@Jaifroid I think moving the ToC button to the corner rather than moving the contents list to the center is more convenient for the user as:

• It doesn't block the main article, the user can have the ToC enabled while reading the ZIM instead of closing it every time they visit the desired section
• It feels more natural to have the table of contents to the side

This can however, have a negative impact on mobile users as the table of contents might look a little clunky.

What are your thoughts on this?

[Jaifroid]

Hmm, I'm not sure. Maybe we should take the cue from Wikipedia here, where the ToC appears on the left (at least for left-to-right languages) as in screenshot. But then the question is whether to replace the Home button, or move it. In some ways it's redundant, as we have a Home link in the top bar. But maybe we can try out putting ToC far-left, then Home, then the rest.

I also wonder if it would be better to use the rectangle-list font-awesome icon instead of "ToC", so that we don't have to deal with translations. You can see it here: https://fontawesome.com/icons/rectangle-list?s=solid

[Greeshmanth1909]

I've added the icon and moved the list to the left, the coloring is a little off though.

[Greeshmanth1909]

All my attempts at manually matching the color of the icon to the rest were futile, obviously. Is there a workaround?

[Greeshmanth1909]

The provided link led to the latest version (6.x.x) of fontawesome, it seems like we're using version 5.15.4. As I was initially trying to use the class based representation from the latest version, it didn't work. This new icon is from one of the older versions and works as expected.

[Jaifroid]

That looks better, thanks! Sorry about having misled you. We are indeed currently using an older Bootstrap, but there is a PR in progress to upgrade it. I think your PR is likely to be ready first. Nevertheless, at last for now, this icon works well. I'll make some further comments in the code.

[Greeshmanth1909]

The clean up messed up the functionality. I'll fix it now.

[Jaifroid]

I haven't done a proper code review yet, as I realized we need to add some code to dismiss/hide the ToC if the user clicks outside of it, or it otherwise loses focus. I suggest you experiment with adding a 'blur' event listener to the dropup, and simply hide the ToC in that event:

dropup.addEventListener('blur', function () {
    ...
});

NB, this needs testing, because it's possible that focus is lost when the document is scrolled to the selected heading, though I don't think that should be the case.

I think this should take care of the other issue, which is that currently the old ToC remains in place if a user navigates to a new article, and it doesn't get rebuilt when the new article is loaded. However, if the blur function works, then navigating to a new article will hide the ToC anyway, and it will get rebuilt when the user clicks on the button.

[Jaifroid]

I also noticed this issue when testing the display in a mobile format in dev tools (the ToC gets squashed up against the left margin -- see screenshot). You may need to add a margin instead of padding, or else use !important in the style rule to ensure the padding is respected even when the display goes into responsive mode.

Ultimately, the style rules will need to be moved to app.css, but for now you can keep them inline while you experiment and iron out bugs. But you should be aware that there is a section in app.css which alters some styles when the width is less than a certain number of pixels. However, I think it's Bootstrap that's displaying things differently on narrow screens rather than our CSS, so an !important might do the job to force the padding to display.

[Greeshmanth1909]

I won't be able to get to this until Sunday as I have exams.

[Jaifroid]

Of course, no rush. Good luck!

[Greeshmanth1909]

@Jaifroid What function gets called when a user clicks on a link that takes them to another article?

[Jaifroid]

@Jaifroid What function gets called when a user clicks on a link that takes them to another article?

In Safe Mode (called jQuery mode in the code), listeners are attached to each link in an article. In SW mode, there is no attached function: the Service Worker catches the browser's Fetch request and diverts it to fetch an article from the ZIM.

Are you looking to tear down the old ToC on new article load? I think that would be a bit complicated, and unnecessary. Simply rebuild it when a new article is loaded. All you have to do is ensure the current ToC gets hidden if it loses focus, for which I think the blur event is easiest.

[Greeshmanth1909]

The list disappears as expected but the auto scroll stopped working. Is this because of conflicting eventListeners for blur and click for the list elements?

[Jaifroid]

The list disappears as expected but the auto scroll stopped working. Is this because of conflicting eventListeners for blur and click for the list elements?

Without testing it, which I can't do just now, it's hard to speculate, but I do notice that you seem to have added the blur event to the button itself, which may not be ideal. Try adding it to the ToCList, as that's what needs to blur, not the button.

[Greeshmanth1909]

I added a timeout within the existing event listener and everything is working as expected.

[Jaifroid]

Hi @Greeshmanth1909 With your latest code, I'm seeing a couple of problems: 1. I have to press the ToC button twice before it responds; 2. Clicking on one of the links fails to scroll the article. Maybe the timeout has introduced a race condition? It would probably be better to solve it without a timeout if at all possible, because timeouts tend to work differently on different devices or under different conditions.

[Greeshmanth1909]

I've noticed that the button needs to be clicked twice only for the first time after a new zim is opened, the toc opens with a single click after this.

[Jaifroid]

Yes, that is my experience too. I forgot to mention that.

[Jaifroid]

@Greeshmanth1909 No rush, but I just wanted to check that you're not awaiting anything from me on this PR.

[Greeshmanth1909]

@Jaifroid It looks like the list element is never focused. The focus shifts between the button, the main article and the iframe. Keeping this in mind, there are three instances I can think of in which the list element has to be hidden:

1. User clicks on the heading they want to checkout, this line executes after the heading is scrolled into view and the element disappears as expected.
   ToCList.style.display = 'none';
2. When the user clicks away from the displayed list.
3. When the user clicks on another link that takes them to a different page.

In both the above mentioned scenarios, the button looses focus. As you've mentioned earlier, the blur event listener can be used to hide the list once the button looses focus. This caused a problem with the automatic scrolling as the button also looses focus when one of the toc contents is clicked.

The timeout approach solved both the problems. I think the present inconsistency regarding the automatic scroll can be addressed by increasing the timeout.
The other ways I found online for this involved the use of JQuery. The timeout approach seemed simpler.

[Jaifroid]

@Greeshmanth1909 You're right that a timeout is much preferable to reintroducing jQuery (which it took a lot of work to eliminate!). Please do try increasing the timeout, or else check what the solution used in Kiwix PWA is (I'm sorry, I can't remember without looking at it in detail).

[Jaifroid]

Changes are discussed in Conversation thread - some functionality still a bit erratic.

[Greeshmanth1909]

Will get this done by this weekend.

[Greeshmanth1909]

I've increased the wait time to hide the list element when it looses focus and decreased the wait time for scrolling. It seems to be working fine.

[Jaifroid]

@Greeshmanth1909 Thanks, but I'm still seeing the issues in screen capture below. To summarize: 1. I still have to click twice on the ToC icon; 2. Selecting a heading doesn't actually jump to the heading.

ToC_issues.mp4

To be clear, I double-checked that I pulled all your changes. Possibly when you merged main into your branch, it might have reintroduced issues?

If it works for you, and not for me, it's likely your timeouts introduced a race condition. If I remember rightly, in the PWA, I set up the ToC every time a page is loaded, so that it is already available when the user clicks the ToC button and doesn't have to be composed. Of course it should be possible to compose it on click time instead, but your current code appears to have a race condition, which is likely what is causing the need to click twice. Adding timeouts is not a good solution to such race conditions, because you can never be sure to allow enough time for very slow devices, and you don't want to slow it down for people with fast devices. An alternative would be to use a Promise or Async code to get the headings. It would be much safer than a timeout.

So, I think you have two options: 1. Compose the ToC at page load time (i.e., once the HTML has been injected into the iframe DOM); 2. Compose the ToC with async code when the user hits the button, if your attempts to use synchronous code are not working.

Of course there could be some other issue why the ToC doesn't display on the first click. Please debug and test carefully.