Javascript API for toc

[bakshiutkarsha]

should resolve #596

[codecov]

Codecov Report

Merging #1237 into master will increase coverage by 0.03%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##           master    #1237      +/-   ##
==========================================
+ Coverage   68.80%   68.84%   +0.03%     
==========================================
  Files          25       25              
  Lines        2199     2208       +9     
  Branches      420      421       +1     
==========================================
+ Hits         1513     1520       +7     
- Misses        514      517       +3     
+ Partials      172      171       -1     

Impacted Files	Coverage Δ
src/config.ts	100.00% <ø> (ø)
src/util/misc.ts	73.26% <55.55%> (+0.21%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87677dc...c7c6068. Read the comment docs.

[kelson42]

@Jaifroid @automactic @macgills This is the really early iteration to create a JS API. I have chosen the TOC API because it is I believe the most simple thing. I would like to have you to join this journey so we secure what is done here is helpful/usable to Kiwix reader' devevelopers. The first thing I would like you to secure with @bakshiutkarsha is that the implementation is correct and work at least as your one in your respective readers.
@mgautierfr From you I would like that you help to decide where to save this (or these) content in the ZIM file. The question of the API to access the content is as well still open.

[Jaifroid]

@kelson42 @bakshiutkarsha Happy to troubleshoot/test from the Kiwix JS perspective. Just looking at the code, as this is intended to run in any supported browser/environment, the code should be re-cast as ES5. We need to support IE11 as a minimum, and 'async' won't work in older browsers.

I don't know the API spec, but as far as I can see, none of the code inside the async blocks actually runs asynchronously, so there is nothing to wait for. I think the same code can simply be written as a set of functions.

[Adding @mossroy to this thread.]

[kelson42]

@Jaifroid we can define what ever we need as API, this is the coal of the PR discussion around TOC. Maybe you have your own piece of code in Kiwix-js you would like to advert here?

[Jaifroid]

OK, thanks! In that case, we'll also need to polyfill Element.closest().

[Jaifroid]

Polyfill here: https://github.com/jonathantneal/closest

Also, IE11 can't use forEach on a querySelector node list. Instead you have to use the Array.prototype.slice.call(list).forEach() syntax or else [].slice.call(list).forEach().

[Jaifroid]

Finally, I notice this uses module.exports, which could be an issue depending on whether you're expecting the browser to import the code, or you're going to ship it bundled with webpack or similar. And if you're going to ship it bundled, then babel will handle the polyfills needed for IE11 and other browsers, so my comments above wouldn't apply in that case.

[mgautierfr]

I will say nothing on the js code, I'm not a expert here. (Except that I'm stuck in a js world where async and module were not existing, old browsers are probably stuck with me so we must be careful about the js we use)

To be complete here, do we agree that :

• The js code will be loaded "somehow" (to define) in the page.
• Client applications (android, ios, desktop) will call functions to get the toc or move to the right position.

I have few questions :

Should we use a js method to change the location ? If a client can get the whole toc of the archive, it could change the location of the page directly (a probably better than relying on a embedded js).

The following script has the advantage to be article independent, the TOC itself is inferred from the html content. But we could also standardize the html content to allow the client to found the TOC itself (not sure it is a good idea, but for brainstorming purpose, it is good to have it in mind).
We could simply add a json dict in each article with the TOC, client would simply have to get it.
(This json dict could be generated with a independent script, but the "api" would be the dict, not the script).

It worth it to compare this with the comment kiwix/libkiwix#322 (comment) the issue kiwix/libkiwix#392

[kelson42]

* The js code will be loaded "somehow" (to define) in the page.

Yes, this is the idea as the reader has control over the HTML widget/DOM. The operation should be as easy as possible and we will need a solution at libzim/libkiwix to make the retrieval of this JS API code as easy/clean as possible.

* Client applications (android, ios, desktop) will call functions to get the toc or move to the right position.

Yes, anything after retrieval of JS API code is full in hand of ZIM reader.

Should we use a js method to change the location ? If a client can get the whole toc of the archive, it could change the location of the page directly (a probably better than relying on a embedded js).

I have not clear opinion on that.

The following script has the advantage to be article independent, the TOC itself is inferred from the html content. But we could also standardize the html content to allow the client to found the TOC itself (not sure it is a good idea, but for brainstorming purpose, it is good to have it in mind).

This is what happens for the moment.

The JS API allows to:

• Not have to reimplement again & again the same logics in readers
• Allow ZIM creator to superseed default implementation and do things differently

We could simply add a json dict in each article with the TOC, client would simply have to get it.
(This json dict could be generated with a independent script, but the "api" would be the dict, not the script).

This is how EPUB works. The JS API is a more flexible/leaner approach to achieve the same result.

[mgautierfr]

The JS API allows to:

    Not have to reimplement again & again the same logics in readers
    Allow ZIM creator to superseed default implementation and do things differently

Yes, I agree. I would put the js code in the zim file itself. And make article add it in the "normal" way.
(Or not have js at all and simply hard code the json dict in the article).

This is how EPUB works. The JS API is a more flexible/leaner approach to achieve the same result.

How it is better ?

[kelson42]

This is how EPUB works. The JS API is a more flexible/leaner approach to achieve the same result.

How it is better ?

• Integrates within the standard JS API (no other concept)
• No additional storage in 99% case
• Nothing to do in normal case fore the scraper
• Ability to handle special cases with only a little storage cost

[kelson42]

Yes, I agree. I would put the js code in the zim file itself. And make article add it in the "normal" way.
(Or not have js at all and simply hard code the json dict in the article).

The JS API is in each/the ZIM.

[kelson42]

Please see comments (mostly from @Jaifroid)

[mgautierfr]

If it is js that generate the dict :

Integrates within the standard JS API (no other concept)

Which one ? We are currently defining it. We could define that a dict have to be present.

No additional storage in 99% case

The js could generate the dict at runtime (read time).

Nothing to do in normal case fore the scraper

If this is to the js to generate it, no more work for the scraper, at least not more than add a js with a specific api in the zim file wi

Ability to handle special cases with only a little storage cost

Same if it is the js creating the dict.
For really specific case, we could store directly the dict without have to create a one time script.

[Jaifroid]

I agree generating the TOC dynamically from the article headings, rather than having it as an extra json item is the simplest generic approach that doesn't require any extra storage.

Apart from that, I think it's important to decide now how the script gets attached to the page. Since MWoffliner is written in Typescript, I believe, you probably have a toolchain that compiles TS down to JS for Node. A similar toolchain can be used to compile ES6 down to ES5 and below, and automatically bundle the needed polyfills for all API scripts as a single optimized JS file - e.g. kiwixAPI.js. Webpack + babel will do this.

Is this what's intended? It would seem the most sensible approach, as it will support even IE11.

(It would be great not to have to bundle jQuery in the ZIM as at present! It's redundant with babel.)

[bakshiutkarsha]

@kelson42 @Jaifroid For now, I have exposed the TableOfContent class in the window object, so anyone can access it from there.

Also, I am bundling the kiwixAPI.js file after Zim gets created, we can separate that out as well. Let me know your thoughts on that?

[kelson42]

@kelson42 @Jaifroid For now, I have exposed the TableOfContent class in the window object, so anyone can access it from there.

Also, I am bundling the kiwixAPI.js file after Zim gets created, we can separate that out as well. Let me know your thoughts on that?

@bakshiutkarsha There is many things which I would like to change with your PR:

• Your PR makes many assumptions about the content. I understand you make this PR based on a Mediawiki Content, but what you are building here is a generic JS API which should as far as possible with any HTML. Relying on data-level attribute is not possible for example
• I don't think we want to have an object here, except if there is an excellent to make it statefull. It should be stateless. The array returns should contained the CSS selector to find again the node. That way it should be possible to have a stateless solution.
• I want all the function in the namespace "zim::toc"
• Your code should work will all kinds of <H> nodes.. not only <h2>
• I hardly see how import could work in that context. @Jaifroid Do you have any idea to have a good system to handle dependencies? The user should typically load within the DOM the needed API only with one call!

[kelson42]

Need changes

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[Jaifroid]

@kelson42 @bakshiutkarsha Is there any way we can get this finished? We were very very close!

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[kelson42]

@Jaifroid I hope so soon. This is part of 1.12 milestone (so in November) with a few other ticket. Hopefully @MananJethwani will be able to help here to complete.

[kelson42]

I have rebased the project on git master.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.