Skip to content

Added ability to specify tag names in selectors#4633

Merged
avimehta merged 5 commits intoampproject:masterfrom
avimehta:bugfixes
Aug 24, 2016
Merged

Added ability to specify tag names in selectors#4633
avimehta merged 5 commits intoampproject:masterfrom
avimehta:bugfixes

Conversation

@avimehta
Copy link
Copy Markdown
Contributor

Roll forward #4617 with bug fixes and tests.

First commit is same as #4368
Second commit includes all the bugfixes.
Third commit adds the tests.

/to @dvoytenko

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change the message please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

@dvoytenko
Copy link
Copy Markdown
Contributor

LGTM with few comments.

avimehta and others added 4 commits August 23, 2016 17:26
* 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
Copy link
Copy Markdown
Contributor Author

ptal. I had to make substantial changes to test to accommodate for using .assert. Another look at the changes to make sure I am not doing anything stupid.

@avimehta
Copy link
Copy Markdown
Contributor Author

@dvoytenko I have been seeing these tests fail on some of the runs. ANy idea why this is happening? The tests pass if I rerun them. https://travis-ci.org/ampproject/amphtml/builds/154618433

@dvoytenko
Copy link
Copy Markdown
Contributor

@avimehta Test failures appear to be unrelated. But please ping Erwin if you keep seeing them.

@dvoytenko
Copy link
Copy Markdown
Contributor

LGTM

@avimehta avimehta merged commit ab9addc into ampproject:master Aug 24, 2016
@avimehta avimehta deleted the bugfixes branch August 24, 2016 22:41
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. (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