Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle unload #7001

Merged
merged 2 commits into from
Jan 12, 2017
Merged

handle unload #7001

merged 2 commits into from
Jan 12, 2017

Conversation

chenshay
Copy link
Contributor

close #6995

@chenshay chenshay requested a review from dvoytenko January 11, 2017 23:26
* Notifies the viewer when this document is unloaded.
* @return {Promise<*>|undefined}
*/
handleUnload(messaging) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be @private with _ suffix.

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


/**
* Notifies the viewer when this document is unloaded.
* @return {Promise<*>|undefined}
Copy link
Contributor

Choose a reason for hiding this comment

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

@param missing.

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

@@ -68,6 +77,9 @@ export class AmpViewerIntegration {

viewer.setMessageDeliverer(messaging.sendRequest.bind(messaging),
dev().assertString(this.unconfirmedViewerOrigin_));

listenOnce(
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests needed. Might be of help - we expose window.eventListeners for testing purposes in realWin and fakeWin fixtures. E.g. you should be able to do something like:

win.eventListeners.fire({type: 'unload'});

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

@chenshay chenshay merged commit e56601a into ampproject:master Jan 12, 2017
@chenshay chenshay deleted the unload branch January 12, 2017 22:46
rpominov pushed a commit to yandex-pcode/amphtml that referenced this pull request Jan 20, 2017
* master: (310 commits)
  Update csa.md to remove non-required parameters (ampproject#6902)
  Add notes about requesting ads ATF and link to demo (ampproject#7037)
  Remove whitelist for lightbox scrollable validator (ampproject#7034)
  Delegate submit events until amp-form is loaded  (ampproject#6929)
  Moves closure sha384 into a new extension amp-crypto-polyfill for lazy load (ampproject#7006)
  Refactor observables in viewer-impl into a map object (ampproject#7004)
  resizing of margins (ampproject#6824)
  Use URL replacer from embed for pixel (ampproject#7029)
  adds support for Gemius analytics (ampproject#6558)
  Avoid duplicating server-layout (ampproject#7021)
  Laterpay validator config (ampproject#6974)
  Validator rollup (ampproject#7023)
  skeleton for amp-tabs (ampproject#7003)
  Upgrade post-css and related packages to latest (ampproject#7020)
  handle unload (ampproject#7001)
  viewer-integr.js -> amp-viewer-integration (ampproject#6989)
  dev().info()->dev().fine() (ampproject#7017)
  Turned on experiment flag (ampproject#6774)
  Unlaunch ios-embed-wrapper for iOS8 to avoid scroll freezing issues (ampproject#7018)
  Add some A4A ad request parameters (ampproject#6643)
  ...
jridgewell pushed a commit to jridgewell/amphtml that referenced this pull request Jan 31, 2017
* handle unload

* fix+test
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
* handle unload

* fix+test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

amp-viewer-integration extension handle unload
2 participants