✨ Add CONTAINER and CONTAINER_TOKEN variable support to amp-analytics. #13499
Conversation
modified: extensions/amp-analytics/0.1/vendors.js modified: src/service/url-replacements-impl.js
|
/to @lannka /cc @calebcordry Who's been working with this recently IIRC.
|
| @@ -513,6 +514,21 @@ export class GlobalVariableSource extends VariableSource { | |||
| return this.getStoryValue_(storyVariables => storyVariables.pageId, | |||
| 'STORY_PAGE_ID'); | |||
| }); | |||
|
|
|||
| this.set('CONTAINER_TOKEN', () => { | |||
| let fragment = getFragment(this.ampdoc.win.location.href); | |||
There was a problem hiding this comment.
This needs to check location.originalHash, we wipe the viewer params from the location.href (and therefore location.hash) on load.
Better yet, you can use viewer's #getParam to get these without doing a query parse.
| this.set('CONTAINER_TOKEN', () => { | ||
| let fragment = getFragment(this.ampdoc.win.location.href); | ||
| if (!fragment) { | ||
| return ''; |
| if (!fragment) { | ||
| return ''; | ||
| } | ||
| let args = parseQueryString(fragment.substring(1, fragment.length)); |
There was a problem hiding this comment.
It'll parse with the # symbol.
| } | ||
| return args.ct; | ||
| }); | ||
| this.set('CONTAINER', () => { |
There was a problem hiding this comment.
Likely we'll need better naming.
| @@ -513,6 +514,21 @@ export class GlobalVariableSource extends VariableSource { | |||
| return this.getStoryValue_(storyVariables => storyVariables.pageId, | |||
| 'STORY_PAGE_ID'); | |||
| }); | |||
|
|
|||
| this.set('CONTAINER_TOKEN', () => { | |||
There was a problem hiding this comment.
what is the use case of this?
we can probably make this a macro function that takes an argument, like we have for QUERY_PARAM('ct'), we can have FRAGMENT_PARAM('ct')
|
@RJSumi are you still on this? Note that @jridgewell is doing a bunch of refactoring on the same file. (merge conflicts :-)). |
|
As discussed offline, @RJSumi will continue in a new PR. |
Please advise on how to better not %-encode CONTAINER, and what kind of unit testing would be good for this, etc.
Fixes #13498