selfthinker / dokuwiki_template_writr

Bold, minimalist and modern DokuWiki template, converted from the Writr WordPress theme
https://www.dokuwiki.org/template:writr
GNU General Public License v2.0
50 stars 16 forks source link

may I ask some pointed questions about skip-link-focus-fix.js #26

Open waffledonkey opened 4 years ago

waffledonkey commented 4 years ago

if jQuery was reassigned to $ in the IIFE on line 1, why is it referenced as jQuery (twice) on line 3 ?

on line 6, isn't <object> deprecated and shouldn't the list include <details> and <iframe> which I understood were focusable. I believe [contenteditable] could also be in the list, but perhaps not in your specific use case.

also on line 6 (and line 14) was it a stylistic choice to negate is() versus using not()? I'm wondering if there's a performance advantage or something.

why do lines 25 and 26 use document.location.hash but line 31 uses window.location.hash?

what is line 31 doing? it looks like it's prepending # to the hash after removing # and I'm wondering what the value in that is. why remove a thing just to put it right back; what is the use case you are solving for?

related, why assign hash to a variable if you're going to immediately use it and never use it again?

also -- sort of related -- instead of using a selector on lines 26 and 32 to find an element and then pass that element to the focusOnElement method where you then have to test length, why not just pass the selector string to focusOnElement and let it find the element? it seems like you're doing work in two places that could have been done in one.

something like:

function focusOnElement(selector) {
  var $element = $(selector).first();

I suppose technically you'd still be calling focus () on a non-element (which quietly fails) and could have exited faster, but checking .length seems so 2015.

Thank you for your time and consideration

LouisOuellet commented 5 days ago

Please read devel:javascript