Refactored doc-switcher.js
#1835
Conversation
- Simplified code - Stopped using jQuery - Moved refactored code to `app.js` This is the first in a series of patches that will move JavaScript code out of `require.js` modules and into a single file while also refactoring. This patch should bring no user-facing changes.
There was a problem hiding this comment.
Should this file be renamed? I wasn't sure what to call it.
| @@ -0,0 +1,5 @@ | |||
| document.querySelectorAll('.doc-switcher li.current').forEach(function (el) { | |||
There was a problem hiding this comment.
This will run on all pages, but in my testing, it seems to be about twice as fast as the old hasClass method from main.js. So I think we're safe here.
| // Make version switcher clickable for touch devices | ||
|
|
||
| self.switchers.find('li.current').on('click', function () { | ||
| $(this).closest("ul").toggleClass('open'); |
There was a problem hiding this comment.
I used the parent element in the new code instead of trying to find the "closest" ul element. The li elements we are looking for should always be direct children of the ul element that needs the class.
|
I suppose we could try to replace this JavaScript with a |
| // Toggle persistent display of documentation version and language options if | ||
| // the element is clicked |
There was a problem hiding this comment.
Should we keep a comment like this? It may be valuable to have a comment for each bit of behavior that goes into this file. I don't feel strongly about it.
app.jsThis is the first in a series of patches that will move JavaScript code out of
require.jsmodules and into a single file while also refactoring.This patch should bring no user-facing changes.
To test, ensure that the functionality of this component on documentation pages still works as expected:
If the "Language" or "Documentation version" elements are clicked, it should toggle them remaining open when not hovered over. They will get a green border when activated.
Part of #1827