[toc2] make toc entries collapsible #1031

jcb91 · 2017-07-23T15:53:46Z

Note, in order to make this work neatly, I have also:

changed how the toc cell is rendered (it now uses the same DOM structure and classes as the sidebar, rather than its previous set of un-nested divs with classes for padding levels)
altered how toc link ids are generated and handled:
- for the semi-unique toc id, don't replace any special characters. By handling the search correctly, we can avoid jquery issues with special characters like ., [, etc. In addition, this prevents potential confusion between e.g. sections 1.13 and 11.3, which would previously have both have been translated to the id as 113.source
- keep the original id as the href for all links (previously, the sidebar links used modified id, which cell links used original ids in order not to break exporting), and store the semi-unique modified id as html attribute data-toc-modified-id. Use the toc.js-specified click handler to ensure that we actually select the modified id

I'm reasonably confident this all works ok from my limited testing, but I'd definitely welcome others' testing, and in particular, anything @jfbercher might have to say :)

also remove redundant css for hovering over items so they don't flicker :/

also: * rework toc link highlighting to work with new-form ids * remove collapsible_headings' now-redundant toc click handler, since toc's own handler selects the cell, which is enough to get ch to reveal it

* add collapse controls after writing toc cell html, so they don't show up as useless in html saveas page * explicitly write spacing for number labels, so the html doesn't need css file to add it as a :before pseudoelement * write toc cell heading margin as a per-element css rule, so html doesn't need css file * fix removal of execute highlight class

jfbercher · 2017-07-24T12:07:19Z

@jcb91
I just say WOW!!

I spent a part of the afternoon yesterday investigating how I could do that, but this is now done and very well done!
Just a couple of minor remarks before testing (I am eventually in vacations and walking through France).

I removed the dots from ids because it was causing (or just was reporting -- don't remember well) causing problems with jquery https://stackoverflow.com/questions/9930577/jquery-dot-in-id-selector May be this is no more the case with recent jquery
for .lev classes, I know there are better ways to do the things, but I remember that some people rely on it for displaying their (old) generated html documents. Perhaps shall we keep the definitions in the css (which is read from the jupyter_contrib repo); or the tpl
For collapsing working together with colapsible_headings, perhaps that both extensions can trig an event, like 'toc_collapsing' or 'header_collapsing' and pass the id of the header being clicked? The behaviour can be made configurable via a boolean 'If toc extension is prensent, also collapse headings in toc' and 'if collapsible_heading extension is present, also collapse sections in notebook'

Again. Wow!

jcb91 · 2017-07-24T14:54:40Z

(I am eventually in vacations and walking through France)

Sounds nice, enjoy! 😄

I removed the dots from ids because it was causing (or just was reporting -- don't remember well) causing problems with jquery May be this is no more the case with recent jquery

Yes, I saw references to that in comments, and as far as I can tell, it's still an issue when using jquery's string selectors: Jquery will interpret a . to signify a class, a [ to specify an attribute, etc. as in css rules. However, the browser's document.getElementById method has no such limitation, so we can leave special characters in the id as long as we use that. In addition, I'm now searching for links with a specified data-toc-modified-id attribute anyway, which I do with jquery's filter rather than a string selector for the id.

for .lev classes, I know there are better ways to do the things, but I remember that some people rely on it for displaying their (old) generated html documents. Perhaps shall we keep the definitions in the css (which is read from the jupyter_contrib repo); or the tpl

Ah, right, I hadn't thought of that! Yes, in that case, I think we should leave them in the css for backward compatibility. They don't need to be in the template though, as they won't be needed for newly-exported versions.

For collapsing working together with colapsible_headings, perhaps that both extensions can trig an event, like 'toc_collapsing' or 'header_collapsing' and pass the id of the header being clicked? The behaviour can be made configurable via a boolean 'If toc extension is present, also collapse headings in toc' and 'if collapsible_heading extension is present, also collapse sections in notebook'

Yes, that sounds like a good plan, to have a setting like "sync toc items' collapsed state with collapsible headings nbextension" in toc2 config. We don't really need a setting for each extension, as then we run into priority problems 😉 I think the simplest way to get it working would be to have collapsible_headings fire an event when collapsing/uncollapsing a heading, something like collapse.HeadingCell and uncollapse.HeadingCell, passing the relevant cell as data. Then toc2 can (un)collapse the toc to match. For the opposite direction, we just add an optional bit in the toc callback to find the heading cell via the modified id as we do for a go-to click, and then call the toggle_heading method (which ch will export). I'll have a stab now.

we pass new events, either through Jupyter's events object, or in a non-live notebook, through an ojbect at `window.events`, which we create if necessary.

jcb91 · 2017-07-28T10:43:00Z

We don't really need a setting for each extension, as then we run into priority problems

Having thought about this more, I think I see what you were getting at, in that you might want the sync in one direction but not the other. I've added separate options for each direction.

Unfortunately, non-live notebooks don't have the jupyter events module defined, so I've had to add a shim whereby we create an equivalent object at window.events if necessary. I hope that makes sense. I think this now all works ok...

jfbercher · 2017-07-31T16:56:05Z

@jcb91 All that seems really great! What a nice idea to have define your own events object.
I will try to explore and test all that this week or so.

jfbercher · 2017-08-07T10:46:57Z

I have finally reviewed and tested the PR.
It is indeed very nice, I like it! 🥇

I just propose some small adaptations:

3e509cd Only add collapse controls to items with descendants
I tested synchronization with collapsible headings in both ways. It seems that collapse_by_id function must be accessible from outside the main function for collapsible --> toc to work. Addressed in a3172d4
collapse_to_match_collapsible_headings value was not updated from the configurator because the value stored in notebook metadata was preferred. Changed in 6e7e562 but probably we must think of a better way to process parameters that must be always be taken from system and those that can be fixed per notebooks
finally, I added a span tag to embed the full item in toc, which enable to highlight the full line instaed of just the html link. 1d3a820
I also tested the export to html and it seems to work as it should.

Really nice @jcb91!. Thanks
I think we can merge if you agree with the small modifications above.

The former is needed for collapse_by_id to be defined in callback_toc2_collapsible_headings, the latter just for consistency/neatness

…tem, not from notebook metadata

jcb91 · 2017-08-15T15:47:34Z

I have finally reviewed and tested the PR.
It is indeed very nice, I like it! 🥇

Haha great, thanks!

I just propose some small adaptations:

Ok, all of these make sense to me.

I've rebased the changes to simplify the now rather-tangled commit history, and made a couple of extra very small changes as part of the rebase:

removed some extra css rules about menu padding, which seem to have got added accidentally
and add a missing a->span alteration for the custom css
moved all function definitions outside the main table_of_contents function, for clarity.

but then I think we should be good to merge, assuming I haven't broken anything with my rebase 😉

jfbercher · 2017-08-16T12:56:46Z

Tested this morning. It is great!

I would have a further modification. I realized yesterday that the toc links, though working in Chrome/Chromium did not work anymore in Firefox (Linux/last version). This may be related to the point mentioned in some docs :

"However, bubbling of a click event will not cause an element to initiate navigation as if a real mouse-click had been received."

eg here or here. A possible solution to simulate the click using the mouse event is mentioned here. Since all the links are on the same page, I suggest to the scrollIntoView method to scroll the element with the given id into view.

 document.getElementById(trg_id).scrollIntoView(true)

I have tested with this modification and it works at least on Chrome/Firefox/Gnome web browser, both in live notebook and after html export (just edited the html to load the current toc2.js on disk). In addition, I think that this has good chances to fix #1023

I have also updated the README.
Can you check that @jcb91 and give the (hopefully last) "go" for merging?

…ids (issue with Firefox)

jfbercher · 2017-08-16T16:17:54Z

updated toc2.tpl to include font-awesome; otherwise carets were not displayed in firefox when exporting to html
updated again main.css. I think that there were things lost somewhere in the various commits/rebase. Check it @jcb91 , there are certainly things that can be improved.

jcb91 · 2017-08-16T17:46:46Z

I would have a further modification. I realized yesterday that the toc links, though working in Chrome/Chromium did not work anymore in Firefox (Linux/last version). This may be related to the point mentioned in some docs

Good catch, good fix! 👍

I have also updated the README.

👍 😄

updated toc2.tpl to include font-awesome; otherwise carets were not displayed in firefox when exporting to html

Another good catch, good fix! 👍

updated again main.css. I think that there were things lost somewhere in the various commits/rebase. Check it @jcb91 , there are certainly things that can be improved.

This, I'm less certain about, though likely it's partly just that I haven't understood. I've added a few comments with questions for clarifications

Can you check that @jcb91 and give the (hopefully last) "go" for merging?

Haha, we're so close 😄!

jcb91 · 2017-08-16T17:34:02Z