Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Add .matches() as default choice for browserUtils.matchSelector #189

Merged
merged 1 commit into from
Jul 23, 2015
Merged

Conversation

jdan
Copy link
Collaborator

@jdan jdan commented Jul 23, 2015

Formerly known as matchesSelector(), with many vendor prefixes as seen in this function.
I'm using ADT in a jsdom environment, which doesn't support any vendor-prefixed variations
of this function.

http://caniuse.com/#feat=matchesselector

Formerly known as matchesSelector(), with many vendor prefixes as seen in this function.
I'm using ADT in a jsdom environment, which doesn't support any vendor-prefixed variations
of this function.

http://caniuse.com/#feat=matchesselector
@@ -15,6 +15,7 @@
goog.provide('axs.browserUtils');

/**
* Use Webkit matcher when matches() is not supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this read "Use Webkit matcher when matches() is supported."?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see you were just adding missing documentation, this makes sense to me now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, tried to follow the pattern we had going on here

@ckundo
Copy link
Collaborator

ckundo commented Jul 23, 2015

It'd be nice to find a way to unit test this. LGTM unless that's a typo in the docs.

ckundo added a commit that referenced this pull request Jul 23, 2015
Add .matches() as default choice for browserUtils.matchSelector
@ckundo ckundo merged commit e089df0 into GoogleChrome:master Jul 23, 2015
@jdan
Copy link
Collaborator Author

jdan commented Jul 23, 2015

Thanks!

@jdan
Copy link
Collaborator Author

jdan commented Jul 25, 2015

@ckundo @alice FWIW there was no change to the changelog to accompany this

@alice
Copy link
Contributor

alice commented Jul 25, 2015

Ah, thanks for the heads up. Mind doing a follow up with the Changelog change? We'll get used to this flow eventually...

@ckundo
Copy link
Collaborator

ckundo commented Jul 25, 2015

Sorry, my mistake! Now I know.

On July 25, 2015 6:24:12 PM EDT, Alice [email protected] wrote:

Ah, thanks for the heads up. Mind doing a follow up with the Changelog
change? We'll get used to this flow eventually...


Reply to this email directly or view it on GitHub:
#189 (comment)

jdan added a commit that referenced this pull request Jul 26, 2015
Added changes from #189 to changelog
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants