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

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

Open
waffledonkey opened this issue Feb 2, 2020 · 0 comments
Open

Comments

@waffledonkey
Copy link

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

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

1 participant