doc/template.html: make modules list closable (fixes #50) #56
Conversation
doc/template.html
Outdated
| } | ||
| } | ||
| element.insertBefore(dropdown_link_copy, | ||
| element.querySelector('a').nextSibling); |
doc/template.html
Outdated
| dropdown_link.innerHTML = ' <b>+</b>'; // the space is for padding | ||
| dropdown_link.style.cursor = 'pointer'; | ||
| // hack in an ID name for the module list | ||
| var collapsibleList = document.querySelectorAll('nav ul li > ul > li') |
There was a problem hiding this comment.
no need to assign this to a variable
doc/template.html
Outdated
| dropdown_link.style.cursor = 'pointer'; | ||
| // hack in an ID name for the module list | ||
| var collapsibleList = document.querySelectorAll('nav ul li > ul > li') | ||
| collapsibleList.forEach(function(element) { |
There was a problem hiding this comment.
I think I recall the return value of document.querySelectorAll not actually being an array in some browsers, and hence methods don't work unless you get them from the Array prototype?
There was a problem hiding this comment.
NodeList.forEach() != Array.forEach(). Mozilla documentation, along with other things I've found, say that NodeList guarantees a forEach.
doc/template.html
Outdated
| // write out a basic 'dropdown_link' reference element | ||
| var dropdown_link = document.createElement('a'); | ||
| dropdown_link.setAttribute('class', 'dropdown_link'); | ||
| dropdown_link.onclick = function(e) { |
doc/template.html
Outdated
| (function() { | ||
| // write out a basic 'dropdown_link' reference element | ||
| var dropdown_link = document.createElement('a'); | ||
| dropdown_link.setAttribute('class', 'dropdown_link'); |
There was a problem hiding this comment.
I recall .setAttribute("class" having issues: assign to .className instead.
doc/template.html
Outdated
| } | ||
| dropdown_link.innerHTML = ' <b>+</b>'; // the space is for padding | ||
| dropdown_link.style.cursor = 'pointer'; | ||
| // hack in an ID name for the module list |
There was a problem hiding this comment.
comment isn't relevant any more?
doc/template.html
Outdated
| var collapsibleList = document.querySelectorAll('nav ul li > ul > li') | ||
| collapsibleList.forEach(function(element) { | ||
| // add a clone of the node to prevent lingering element scoping | ||
| // otherwise each time a dropdown is clicked it only does the last one |
There was a problem hiding this comment.
I don't understand this comment.
| collapsibleList.forEach(function(element) { | ||
| // add a clone of the node to prevent lingering element scoping | ||
| // otherwise each time a dropdown is clicked it only does the last one | ||
| var dropdown_link_copy = dropdown_link.cloneNode(true); |
There was a problem hiding this comment.
move this down below the early return?
| return; | ||
| list.style.display = 'none'; | ||
| element.setAttribute('state', 'up'); | ||
| dropdown_link_copy.onclick = function() { |
There was a problem hiding this comment.
I wonder if it would make more sense to have an open and a close function that you swap between?..... maybe not.
There was a problem hiding this comment.
same issue as insertAfter - use in only 1.5 places.
4 similar comments
doc/template.html
Outdated
| dropdown_link.innerHTML = ' <b>+</b>'; // the space is for padding | ||
| dropdown_link.style.cursor = 'pointer'; | ||
| document.querySelectorAll('nav ul li > ul > li').forEach( | ||
| function(element) { |
There was a problem hiding this comment.
This should be indented one level further.
doc/template.html
Outdated
| // write out a basic 'dropdown_link' reference element | ||
| var dropdown_link = document.createElement('a'); | ||
| dropdown_link.className = 'dropdown_link'; | ||
| dropdown_link.innerHTML = ' <b>+</b>'; // the space is for padding |
There was a problem hiding this comment.
If the space is padding then you should use .
But, if it's just padding: why not use the css/style for it? i.e. dropdown_link.style["padding-left"]
There was a problem hiding this comment.
trying to reduce the amount of css required :P
| <script type="text/javascript"> | ||
| (function() { | ||
| // write out a basic 'dropdown_link' reference element | ||
| var dropdown_link = document.createElement('a'); |
There was a problem hiding this comment.
why an a link if it doesn't have a href? Should it just be a button instead?
There was a problem hiding this comment.
button elements add additional CSS. Using a a tag accomplishes what we want with minimal CSS additions.
There was a problem hiding this comment.
okay. I'm curious what the button css required would need to be; but don't worry about it :)
doc/template.html
Outdated
| function(element) { | ||
| var dropdown_link_copy = dropdown_link.cloneNode(true); | ||
| var list = element.querySelector('ul'); | ||
| if (!list || list.querySelectorAll('li').length == 1) |
There was a problem hiding this comment.
instead of list.querySelectorAll(...).length==1 you could use list.querySelector(...)
There was a problem hiding this comment.
The reason I did this was to not hide items that had only one subelement. Either I can remove this entirely or keep it as this; there's no way other than this to detect whether there's n amount of elements.
2 similar comments
|
@daurnimator boop |
|
A few requests:
|
I'm pushing this but it'll probably look a bit weird to some.
Fixed.
I can probably change it to |
There was a problem hiding this comment.
Better :)
However, now you have the list bullet followed by the +/-: could you remove the list bullet? Or alternatively, perhaps the +/- could be the bullet?
Also, the + and - are different widths, so you get some slight movement when expanding/collapsing: could you make it a fixed width (possibly automatically fixed if you make it the bullet)
doc/template.html
Outdated
| $endif$ | ||
| <title>$if(title-prefix)$$title-prefix$ – $endif$$pagetitle$</title> | ||
| <style type="text/css">code{white-space: pre;}</style> | ||
| <style type="text/css">code{white-space: pre;} b{color: #212121;}</style> |
There was a problem hiding this comment.
Instead of changing here, could you set dropdown_link.style.color?
RyanSquared
force-pushed
the
master
branch
2 times, most recently
from
d9c7d6d to
9a49853
Compare
| var dropdown_link = document.createElement('a'); | ||
| dropdown_link.className = 'dropdown_link'; | ||
| dropdown_link.innerHTML = '<b>+</b> '; | ||
| dropdown_link.style.color = "#212121"; |
There was a problem hiding this comment.
double quotes here (single quotes everywhere else)
3 participants
No description provided.