Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[toc2] with github html preview #1023

Closed
bianbian2010 opened this issue Jul 19, 2017 · 10 comments
Closed

[toc2] with github html preview #1023

bianbian2010 opened this issue Jul 19, 2017 · 10 comments

Comments

@bianbian2010
Copy link

Hi~

It seems the section links in the floating/side table do not work in html preview mode example here (links in the static table do work so I think it might be how the section IDs are appended to the link?)

I am very new to Jupyter and html/css so sorry for my ignorance. Please let me know if there is a quick fix I can apply on my own to resolve this issue.

Thanks very much!

@jcb91
Copy link
Member

jcb91 commented Jul 19, 2017

Hmm, it seems like all the sidebar links (which are generated by javascript, and only specify a hash, like #H11-1, not a full uri) are computed by my browser to point to raw.github.com, (e.g. #H11-1 takes you to https://raw.githubusercontent.com/bianbian2010/temp/master/toc2-example.html#H11-1) whereas those on the main page have been specifically filled in with a full uri (e.g. https://htmlpreview.github.io/?https://github.com/bianbian2010/temp/blob/master/toc2-example.html#H11).

Essentially, I think this could be considered a fault of htmlpreview.github.io, rather than toc2.
My guess is that htmlpreview.github.io is doing some magic to process hrefs which are present on the page at load (see htmlpreview.js#L25-L33), but any links subsequently added by javascript (like those in the sidebar) don't get the same treatment. Perhaps it would make sense to file an issue at htmlpreview/htmlpreview.github.com/issues?

@bianbian2010
Copy link
Author

bianbian2010 commented Jul 19, 2017

Thanks very much for the detailed reply!

Is it possible to modify the javascript to generate a full uri? (I can try to make this change on my own if it's not too complicated) I've seen a working sidebar in ahambi/140824-TOC here and just want to try in case the href handling from htmlpreview was intended.

@jfbercher
Copy link
Member

jfbercher commented Jul 20, 2017

yes, I agree with @jcb91's analysis. the bug is more on the htmppreview side.
@bianbian2010 can you try to edit toc2.js, line 26 into

 a.attr("href", window.location.href + '#' + h.attr('id'));

and see if it works?

@jcb91
Copy link
Member

jcb91 commented Jul 21, 2017

Ah, the working example is interesting. I think the difference is that in ahambi/140824-TOC's code (adapted from minrk's), the links are generated by cloning the href of the link in the original header:

h = element.find("div.text_cell_render").find(':header').first();
var a = h.find('a').clone();
new_a.attr("href", a.attr("href"));

and since the header anchor links have already been processed by htmlprevew, the adjusted URI is used for the toc link too. Although really they should be looking for .anchor-link rather than a, since there seem to be two a elements for each header, one of which has only a name with no href.

In contrast, the toc2 from this repo generates its links directly using ids, so it doesn't get the modified versions, leading to the behaviour you describe. I think @jfbercher's solution explicitly adding the window href should work fine. Alternatively, another possible fix would be to adopt the previous method, and modify line 26 to read

a.attr("href", h.find('.anchor-link').attr('href'));

@jfbercher
Copy link
Member

jfbercher commented Jul 21, 2017

Thanks @jcb91 .
@bianbian2010 Actually, for rendering the html with embedded toc, we need the toc2.js to be updated on github. I'll try to push a PR perhaps this week-end. [I think that in the ahambi/140824-TOC example the js is included in the html]

@jcb91 jcb91 changed the title toc 2 with github html preview [toc2] with github html preview Jul 21, 2017
jfbercher added a commit to jfbercher/jupyter_contrib_nbextensions that referenced this issue Jul 22, 2017
@bianbian2010
Copy link
Author

Thank you very much @jcb91 and @jfbercher !

It seems now the side bar link points only to the prepended part not the full uri (e.g. #H11-1 takes you to https://htmlpreview.github.io/#H11-1, where it should be https://htmlpreview.github.io/?https://github.com/bianbian2010/temp/blob/master/toc2-example.html#H11-1). example here

And as @jfbercher said, I wasn't able to implement local changes made to toc2.js (I did the editable install but the change in local file does not take effect for some reason?)

@mohammad7t
Copy link

mohammad7t commented Aug 7, 2017

I think "cloning the href of the link in the original header" as mentioned by @jcb91 is more robust. The query string after https://htmlpreview.github.io/? won't be preserved by current method:

a.attr("href", window.location.origin + window.location.pathname + '#' + h.attr('id'));

@jfbercher
Copy link
Member

@mhsekhavat Sure!
We could add window.location.search to be complete (or may be more simply take the full window.location.href and remove the # part, e.g window.location.href.split('#')[0])

For the cloning version, I am not sure that h.find('.anchor-link').attr('href') does not only return the hash part since it seems that everything in the notebook is defined as relative to the current page.

Anyway there are many changes coming and I think that this question should be reconsidered thenafter.

@jfbercher
Copy link
Member

@bianbian2010
I just looked at htmlpreview.github.com. it seems that there are also issues with both relative and external links. Thinking to that, I was wondering why you don't use the raw git access, which gives https://rawgit.com/bianbian2010/temp/master/toc2-example.html for your example?

jfbercher added a commit to jcb91/jupyter_contrib_nbextensions that referenced this issue Aug 12, 2017
@jfbercher
Copy link
Member

Seems to be solved by #1031. Closing. Reopen if I missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants