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
SCDoc: redesign TOC and menubar again #3831
Conversation
HelpSource/scdoc.js
Outdated
$("#toc").appendTo(toc_container); | ||
|
||
toc_items = document.getElementById("toc") | ||
.getElementsByTagName("ul")[0].getElementsByTagName("li"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz why not use zepto
selector here instead of vanilla js? Can't you just do (like jquery):
$('#toc ul').eq(0).find('li')
Same below with $('#toc_search')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was originally written in vanilla js, and i just haven't bothered to refactor it to use zepto.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz you use zepto
selectors one line above so please, let's either use it or not, but not both
@snappizz what exactly is the benefit to using |
zepto is smaller. full jquery functionality is not needed. |
HelpSource/scdoc.js
Outdated
$("#toc").appendTo(toc_container); | ||
|
||
toc_items = document.getElementById("toc") | ||
.getElementsByTagName("ul")[0].getElementsByTagName("li"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz you use zepto
selectors one line above so please, let's either use it or not, but not both
HelpSource/scdoc.js
Outdated
toc_items = document.getElementById("toc") | ||
.getElementsByTagName("ul")[0].getElementsByTagName("li"); | ||
|
||
document.getElementById("toc_search").onkeyup = toc_search; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz same here
HelpSource/scdoc.js
Outdated
toc_link.on("click", function (e) { | ||
e.preventDefault(); | ||
document.getElementById("toc_search").focus(); | ||
$("#toc").toggle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz same here
Just tried this branch. It's a massive improvement in usability -- excellent work, Nathan! The floating menu bar results in one display glitch. I wouldn't have any idea how to fix it. I could live with this more than I could live with the 3.9 workflow, but I guess it's only a matter of time before some user complains. Jumping to a named anchor positions the anchored text at the top of the window, but the menu bar covers part of the top of the window. So part of the text is hidden.
|
thanks james. i won't have access to my dev environment for the next few days, but i'll use this fix: https://css-tricks.com/hash-tag-links-padding/ |
This commit loads jQuery into the SCDoc system.
This commit: - Restores the floating menu bar in SCDoc. - Fixes print-friendliness (supercollider#3395) - Redesigns TOC to fix an issue with internal links (supercollider#3831) - Unifies appearance of TOC and index submenu - Refactors a lot of code to use jQuery
i've fixed the internal links issue, upgraded from zepto to jquery, and refactored some more old code to use jQuery. there's still some non-jQuery code adjacent to jQuery code. i don't wanna hear about that, there's only so much adjacent refactoring i'm willing to do here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snappizz nice one, thank you :)
My complaints were about some non-jquery code in the middle of some jquery code in your additions, not in the older code, so all looks good now!
ps: ah, I still see the anchor link issue James reported
Signed-off-by: Yvan Volochine <yvan.github@gmail.com>
@snappizz @jamshark70 I pushed a small fix for the padding glitch on anchor links.. hope you don't mind ;) |
thanks yvan, looks good! |
Hi, However, I have found the following:
If you already know, I apologise for writing this. |
@prko Please open an issue rather than commenting here. This PR is closed. BTW I am seeing this same issue. |
a lot of changes here:
cc @jamshark70.