Added a var for Viewer. #1894
Merged
Added a var for Viewer. #1894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -29,6 +29,9 @@ import {userNotificationManagerFor} from './user-notification'; | |||
| /** @private {string} */ | |||
| const TAG_ = 'UrlReplacements'; | |||
|
|
|||
| /** @private {string} */ | |||
| const VIEWPORT_ORIGIN_PARAM_ = 'origin'; | |||
There was a problem hiding this comment.
This should probably be "VIEWER", not a "VIEWPORT"
|
ptal. The test failures seem unrelated. Can't reproduce on my machine. Not sure what is going wrong. |
| Provides an identifier for the viewer that contains the AMP document. Empty string is provided when the document is loaded directly in the browser or if the id is not found. | ||
| For instance: | ||
| ```html | ||
| <amp-pixel src="https://foo.com/pixel?title=VIEWER"></amp-pixel> |
There was a problem hiding this comment.
nit: title=VIEWER -> viewer=VIEWER
|
Not sure why @dvoytenko 's commit got involved here. Will try to fix it but this happened in someone else's PR as well today. |
| @@ -157,6 +164,9 @@ export class Viewer { | |||
| /** @private {?string} */ | |||
| this.messagingOrigin_ = null; | |||
|
|
|||
| /** @private {?string} */ | |||
| this.viewerOrigin_ = null; | |||
There was a problem hiding this comment.
Remove. The definition below is the correct one.
|
Looking good - just a couple more comments. |
This var is returns the hostname found in the 'origin' param of the viewer or the referrer. Fixes ampproject#1493 Review feedback, lint fixes. Review feedback.
|
LGTM. Is this ready to be merged? |
|
Yes sir
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This var is returns the hostname found in the 'origin' param of the
viewer or the referrer.
There are a couple of open issues in this PR. Comments and suggestions welcome.
Fixes #1493