Skip to content

Conversation

@eminaz
Copy link
Contributor

@eminaz eminaz commented Jun 25, 2017

when selecting element by id in selectorParse, use getElementById instead of querySelector for better performance (https://jsperf.com/getelementbyid-vs-queryselector), and this way it also support id that starts with a digit (querySelector doesn’t support that)

…tead of querySelector for better performance (https://jsperf.com/getelementbyid-vs-queryselector), and this way it also support id that starts with a digit (querySelector doesn’t support that)
@dmarcos
Copy link
Member

dmarcos commented Jun 25, 2017

Thanks! Almost ready to go. There's a broken test to fix

@eminaz
Copy link
Contributor Author

eminaz commented Jun 26, 2017

Fixed.

function selectorParse (value) {
if (!value) { return null; }
if (typeof value !== 'string') { return value; }
if (value[0] === '#' && !/[.,[: ]/.test(value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the second check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's to check if the query is '#someId.someClass div[name="b"]' etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the regex

@ngokevin
Copy link
Member

I've updated to fix tests, added tests for selectors that were not accounted for, fixed the regex.

@ngokevin ngokevin merged commit 25e3b9a into aframevr:master Jun 26, 2017
@ngokevin
Copy link
Member

thanks much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants