Combine amp-analytics var docs with var substitutions doc re: #1302 #5576

ghost · 2016-10-13T17:22:48Z

@rudygalfi & @avimehta - I've taken Avi's work and Rudy's suggestion to create a combined list of variable substitutions. (re: #1302, Avi's PR).

A couple things to note:

I didn't include the amp-access-analytics variables as they seemed to be a specialized case. Should they be included in this list?
I included more variables than were described in the original docs, essentially I checked against the vars "list" in vendors.js. Is that okay? Essentially, some vars were described for amp-analytics but not in the var-substitution doc (e.g., scrollHeight, scroll...)

avimehta · 2016-10-13T17:46:21Z

Have not gone through the complete list yet but a few things:

I'll sent out a PR(Update vendors.js #5578) to add the not supported variables to amp-analytics. Feel free to update this PR.
The amp-analytics variables are used by enclosing them in ${} so RANDOM will looks like ${random}. It will be nice to use the full format in the amp-analytics variable name.

avimehta

mostly looks good. I'll defer to @rudygalfi for actual formatting etc.

avimehta · 2016-10-24T21:13:08Z

spec/amp-var-substitutions.md

+
+The tables below list the available URL variables grouped by type of usage. Further down in this document, are [descriptions](#variable-descriptions) of each of the variables, along with example usages.
+
+* [Page and Content](#page-and-content)


Maybe this section is not needed?

Not sure why it's not. Can you clarify why?

my thinking was along the lines that this results in 2 levels of tabels to get to the variables. Given that this is a small table and the links from here go directly below, maybe this is not needed.

But, I am okay with whatever you decide here and will go with that.

I see that but for consistency, I'll leave it. It's possible too that someone many change the order, so it's good to keep it.

avimehta · 2016-10-24T21:17:29Z

spec/amp-var-substitutions.md


-### PAGE_DOWNLOAD_TIME
+### Page View ID


I think this needs to be ####.

avimehta · 2016-10-24T21:20:34Z

spec/amp-var-substitutions.md


-### SOURCE_HOSTNAME
+**Arguments**


In the rendered doc, the 'arguments' and client id look exactly the same and it looks like arguments is just another type of variable. If there any way to change the formatting?

avimehta

lgtm from my side. not sure if rudy wants to review.

the PR does need to be rebased to head to resolve conflicts.

ghost · 2016-11-02T19:01:05Z

@rudygalfi - let me know if it's good to go. Ideally, I'd like to get this out before any more vars are added to the analytics-vars doc :)

rudygalfi · 2016-11-02T23:16:53Z

This is a dream come true. Looks fantastic!

…tion rewrite (ampproject#5982) * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * Initial changes, need to update tests * Start fixing tests * Fix unit test coverage * revert a4a examples Introduce amp-auto-id attribute to AMP element. (ampproject#5973) * Introduce amp-auto-id attribute to AMP element. * Fix tests * Address comments * Address comments * revert amp-live-list.md polling clarification (ampproject#5991) Only report 1% of errors if a page has non-AMP JS (ampproject#5994) Fixes ampproject#5732 Allow for hosted testing to override where third party frame is retrieved from (ampproject#5890) * Allow for hosted testing to override where third party frame is retrieved from * test my own site * fix ava tests Remove unsupported query selector feature (ampproject#5999) Workaround for misbehaving webview viewer (ampproject#6001) A webview viewer from Google is seing origins of the form `www.google.com`. For the time being we accept these as they trigger errors in our error reporting that aren't useful. As this form of origins can only occur with webviews that have full control over the page via JS injection anyway, this change should make no difference for normal web usage. A4A integration tests (ampproject#5812) * First draft of full integration tests for A4A. * First draft of full integration tests for A4A. * Got tests of exception conditions working. Refactored some common stuff to utils.js. * Refactored XDom and friendly iframe tests to helpers. * Added tests. * Resolved merge conflicts. * Centralized visibility check and updated visibility test. * Moved all A4A tests to using central visibility test. * Lint fixes. * Changed custom expectations to local functions. * Replaced stringToArrayBuffer with utf8Encode. * Fixed visibility assertion. * Lint fixes. * Added a comment (partly in order to get Travis to re-run). * Changes in response to reviews. * Rebased and fixed small rebase error. Fixes an issue where ads are not correctly centered on certain platforms. (ampproject#6003) * Centralized creative centering in doubleclick.js, and changed css 'transform' to a more commonly supported property. * Added check for existence and changed to plain old 'transform'. * Now asserting the existence of the container. amp-sticky-ad close button new style (ampproject#5979) * unit style * fix css * fix test * rebase * fix z-index Other JS errors: Use startsWith (ampproject#6006) Fixes ampproject#5994 (comment). Fix log calls without TAGNAME (ampproject#6005) My presubmit was a bit too lenient, and too strict at the same time. refactor amp-ad.css (ampproject#5992) Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 (ampproject#5576) * Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 * Updated non-supported vars + format changes for amp-analytics vars * Updates per Avi's feedback * Updated descriptions for amp-carousel vars add version parameter to AMP.extension signature (ampproject#5989) * add version parameter to AMP.extension signature * apply recommendations * remove TODO Improve error reporting (ampproject#6019) - Include the current `location.hash`. We are missing it, because it is not included in the referrer. - Include previously recorded errors, so it is easier to identify follow on errors. - Kill the 2000 char limit as it isn't important for AMP's target browsers. Include the originalHash (ampproject#6020) We remove the hash in embedding mode, so the original code doesn't actually work. Fix error: "Can't find variable: TextDecoder" (ampproject#6011) * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * Initial changes, need to update tests * Start fixing tests * Fix unit test coverage * revert a4a examples * Include self (window) when verifying TextDecoder/TextEncoder existence * fix upstream conflict * PR feedback * PR feedback

Update ads.amp.html Modify A4A AMP Creative to use ampRuntimeUtf16CharOffsets from validation rewrite (ampproject#5982) * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * Initial changes, need to update tests * Start fixing tests * Fix unit test coverage * revert a4a examples Introduce amp-auto-id attribute to AMP element. (ampproject#5973) * Introduce amp-auto-id attribute to AMP element. * Fix tests * Address comments * Address comments * revert amp-live-list.md polling clarification (ampproject#5991) Only report 1% of errors if a page has non-AMP JS (ampproject#5994) Fixes ampproject#5732 Allow for hosted testing to override where third party frame is retrieved from (ampproject#5890) * Allow for hosted testing to override where third party frame is retrieved from * test my own site * fix ava tests Remove unsupported query selector feature (ampproject#5999) Workaround for misbehaving webview viewer (ampproject#6001) A webview viewer from Google is seing origins of the form `www.google.com`. For the time being we accept these as they trigger errors in our error reporting that aren't useful. As this form of origins can only occur with webviews that have full control over the page via JS injection anyway, this change should make no difference for normal web usage. A4A integration tests (ampproject#5812) * First draft of full integration tests for A4A. * First draft of full integration tests for A4A. * Got tests of exception conditions working. Refactored some common stuff to utils.js. * Refactored XDom and friendly iframe tests to helpers. * Added tests. * Resolved merge conflicts. * Centralized visibility check and updated visibility test. * Moved all A4A tests to using central visibility test. * Lint fixes. * Changed custom expectations to local functions. * Replaced stringToArrayBuffer with utf8Encode. * Fixed visibility assertion. * Lint fixes. * Added a comment (partly in order to get Travis to re-run). * Changes in response to reviews. * Rebased and fixed small rebase error. Fixes an issue where ads are not correctly centered on certain platforms. (ampproject#6003) * Centralized creative centering in doubleclick.js, and changed css 'transform' to a more commonly supported property. * Added check for existence and changed to plain old 'transform'. * Now asserting the existence of the container. amp-sticky-ad close button new style (ampproject#5979) * unit style * fix css * fix test * rebase * fix z-index Other JS errors: Use startsWith (ampproject#6006) Fixes ampproject#5994 (comment). Fix log calls without TAGNAME (ampproject#6005) My presubmit was a bit too lenient, and too strict at the same time. refactor amp-ad.css (ampproject#5992) Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 (ampproject#5576) * Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 * Updated non-supported vars + format changes for amp-analytics vars * Updates per Avi's feedback * Updated descriptions for amp-carousel vars add version parameter to AMP.extension signature (ampproject#5989) * add version parameter to AMP.extension signature * apply recommendations * remove TODO Improve error reporting (ampproject#6019) - Include the current `location.hash`. We are missing it, because it is not included in the referrer. - Include previously recorded errors, so it is easier to identify follow on errors. - Kill the 2000 char limit as it isn't important for AMP's target browsers. Include the originalHash (ampproject#6020) We remove the hash in embedding mode, so the original code doesn't actually work. Fix error: "Can't find variable: TextDecoder" (ampproject#6011) * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * Initial changes, need to update tests * Start fixing tests * Fix unit test coverage * revert a4a examples * Include self (window) when verifying TextDecoder/TextEncoder existence * fix upstream conflict * PR feedback * PR feedback

…ect#1302 (ampproject#5576) * Combine amp-analytics var docs with var substitutions doc re: ampproject#1302 * Updated non-supported vars + format changes for amp-analytics vars * Updates per Avi's feedback * Updated descriptions for amp-carousel vars

Combine amp-analytics var docs with var substitutions doc re: #1302

37ce1ff

avimehta assigned avimehta and rudygalfi Oct 13, 2016

Updated non-supported vars + format changes for amp-analytics vars

e51fa04

erwinmombay mentioned this pull request Oct 17, 2016

Meeting Notes 2016-10-17 #5645

Closed

ghost added the Related to: Documentation label Oct 24, 2016

avimehta suggested changes Oct 24, 2016

View reviewed changes

Updates per Avi's feedback

25fbff8

avimehta approved these changes Nov 2, 2016

View reviewed changes

Merge changes for var docs

c9bf506

ghost mentioned this pull request Nov 2, 2016

Carousel analytics vars cleanup #5956

Closed

Updated descriptions for amp-carousel vars

24f4a4b

Merged addition of counter variable

1ecd4ec

Meggin merged commit 1c89d29 into ampproject:master Nov 3, 2016

ghost deleted the amp-url-vars-substitution branch November 4, 2016 12:16

erwinmombay mentioned this pull request Nov 7, 2016

Meeting Notes 2016-11-07 #6067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine amp-analytics var docs with var substitutions doc re: #1302 #5576

Combine amp-analytics var docs with var substitutions doc re: #1302 #5576

ghost commented Oct 13, 2016

avimehta commented Oct 13, 2016 •

edited

Loading

avimehta left a comment

avimehta Oct 24, 2016

ghost Nov 2, 2016

avimehta Nov 2, 2016

ghost Nov 2, 2016

avimehta Oct 24, 2016

avimehta Oct 24, 2016

avimehta left a comment

ghost commented Nov 2, 2016

rudygalfi commented Nov 2, 2016


		The tables below list the available URL variables grouped by type of usage. Further down in this document, are [descriptions](#variable-descriptions) of each of the variables, along with example usages.

		* [Page and Content](#page-and-content)


		### SOURCE_HOSTNAME
		Arguments

Combine amp-analytics var docs with var substitutions doc re: #1302 #5576

Combine amp-analytics var docs with var substitutions doc re: #1302 #5576

Conversation

ghost commented Oct 13, 2016

avimehta commented Oct 13, 2016 • edited Loading

avimehta left a comment

Choose a reason for hiding this comment

avimehta Oct 24, 2016

Choose a reason for hiding this comment

ghost Nov 2, 2016

Choose a reason for hiding this comment

avimehta Nov 2, 2016

Choose a reason for hiding this comment

ghost Nov 2, 2016

Choose a reason for hiding this comment

avimehta Oct 24, 2016

Choose a reason for hiding this comment

avimehta Oct 24, 2016

Choose a reason for hiding this comment

avimehta left a comment

Choose a reason for hiding this comment

ghost commented Nov 2, 2016

rudygalfi commented Nov 2, 2016

avimehta commented Oct 13, 2016 •

edited

Loading