Skip to content

Added ability to specify tag names in selectors.#4368

Merged
avimehta merged 4 commits intoampproject:masterfrom
avimehta:closest
Aug 18, 2016
Merged

Added ability to specify tag names in selectors.#4368
avimehta merged 4 commits intoampproject:masterfrom
avimehta:closest

Conversation

@avimehta
Copy link
Contributor

@avimehta avimehta commented Aug 4, 2016

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes #4018

@dvoytenko
Copy link
Contributor

@avimehta ready for review?

@avimehta
Copy link
Contributor Author

avimehta commented Aug 4, 2016

The PR is ready for review but it has significant merge conflicts with #4265 and #4072. I wouldn't mind if you waited for those to be merged first.

@dvoytenko
Copy link
Contributor

SG. Please ping when ready.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes ampproject#4018
@avimehta
Copy link
Contributor Author

avimehta commented Aug 9, 2016

@dvoytenko this is ready for review.

* ancestor of the analytics element with that tag name is returned.
*
* @param {string} selector The selector for the element to track.
* @param {!HTMLElement} el Element whose ancestors to search.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just !Element: here and below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Added a property called `selectionMethod`. This property dictates how
the selector is treated while searching for elements.
@avimehta
Copy link
Contributor Author

@dvoytenko ptal

</amp-anim>
<amp-anim src="https://goo.gl/FBCKQO" id="anim-id" width="400" height="225" layout="responsive">
<amp-img placeholder src="https://goo.gl/GjcSkr" width="400" height="225" layout="responsive"></amp-img>
<amp-analytics id="nestedAnalytics">
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: indent is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@dvoytenko dvoytenko added the LGTM label Aug 17, 2016
@dvoytenko
Copy link
Contributor

LGTM with a couple more comments.

@avimehta
Copy link
Contributor Author

@rudygalfi would you mind taking a look at the docs?

@avimehta avimehta merged commit c25827e into ampproject:master Aug 18, 2016
@avimehta avimehta deleted the closest branch August 18, 2016 22:18
@avimehta avimehta restored the closest branch August 18, 2016 22:57
avimehta added a commit to avimehta/amphtml that referenced this pull request Aug 24, 2016
* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes ampproject#4018
avimehta added a commit that referenced this pull request Aug 24, 2016
* Added ability to specify tag names in selectors. (#4368)

* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes #4018
ariangibson pushed a commit to Mixpo/amphtml that referenced this pull request Sep 7, 2016
* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes ampproject#4018
ariangibson pushed a commit to Mixpo/amphtml that referenced this pull request Sep 7, 2016
* Added ability to specify tag names in selectors. (ampproject#4368)

* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes ampproject#4018
mityaha pushed a commit to brightcove-archive/ooyala_amphtml that referenced this pull request Nov 30, 2016
* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

Fixes ampproject#4018
mityaha pushed a commit to brightcove-archive/ooyala_amphtml that referenced this pull request Nov 30, 2016
* Added ability to specify tag names in selectors. (ampproject#4368)

* Added ability to specify tag names in selectors.

If a tag name is specified, an ancestor of the amp-analytics tag with
idetified by the tag name will be used for visibilitySpec.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants