luke-chang / js-spatial-navigation

A javascript-based implementation of Spatial Navigation.
Mozilla Public License 2.0
380 stars 117 forks source link

FIX bug for empty selector in getSectionDefaultElement #51

Closed erwanlfrt closed 2 years ago

erwanlfrt commented 2 years ago

Hi @luke-chang,

Your refacto seems to be correct but in case of an empty selector, an error appears : Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Document': The provided selector is empty.

We just need to check first, like before, if the selector is empty.

Thanks for the refacto ;) Have a nice day !

luke-chang commented 2 years ago

~I don't encounter this error. parseSelector accepts '', undefined and null, and querySelectorAll can be invoked only when defaultElement is a string. Maybe there is a case I'm not aware. Could you provide an example for it? Thanks.~

Oh! I see. My example includes jQuery so it won't fall into querySelectorAll anyway. You are right!

(There should have been some unit tests....)

luke-chang commented 2 years ago

Would you mind fixing it by adjusting parseSelector? What I imagine is as below:

function parseSelector(selector) {
    var result = [];
    try {
      if (selector) {
        if ($) {
          result = $(selector).get();
        } else if (typeof selector === 'string') {
          result = [].slice.call(document.querySelectorAll(selector));
        } else if (typeof selector === 'object' && selector.length) {
          result = [].slice.call(selector);
        } else if (typeof selector === 'object' && selector.nodeType === 1) {
          result = [selector];
        }
      }
    } catch (err) {
      console.error(err)
    }
    return result;
  }
}

Thanks.

erwanlfrt commented 2 years ago

I totally agree with your solution, I have updated the code according to your solution. Thanks.

erwanlfrt commented 2 years ago

Done.