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

Implement amp-install-serviceworker #954

Merged
merged 1 commit into from
Nov 20, 2015

Conversation

cramforce
Copy link
Member

Allows AMP pages to install a ServiceWorker to among other things
implement the pattern described in
https://medium.com/@cramforce/amps-and-websites-in-the-age-of-the-service-worker-8369841dc962

@cramforce
Copy link
Member Author

Fixes #586

*/
class AmpServiceWorkerInstallation extends AMP.BaseElement {
/** @override */
isLayoutSupported(layout) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the default, so you don't have to include it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jmadler
Copy link
Contributor

jmadler commented Nov 17, 2015

In light of #800 - should we rename this component to e.g. amp-install-sw, amp-install-app


### <a name="amp-serviceworker-installation"></a> `amp-serviceworker-installation`

The `amp-serviceworker-installation` component allows installing a [ServiceWorker](http://www.html5rocks.com/en/tutorials/service-worker/introduction/) for the current page.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The article above is a bit better for a 1st time user.

@cramforce
Copy link
Member Author

@jmadler Good point. These are, of course, very different in that this component executes without user action.


The `amp-serviceworker-installation` component allows installing a [ServiceWorker](http://www.html5rocks.com/en/tutorials/service-worker/introduction/) for the current page.

The idea here is that this ServiceWorker runs whenever the AMP file is served from the origin where you publish the AMP file. The ServiceWorker will not be loaded when the document is loaded from an AMP cache.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we clarify which hostname the SW is registered to?

Copy link
Member Author

Choose a reason for hiding this comment

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

That isn't really the right dimension for SW since it uses path based origins.

@dvoytenko
Copy link
Contributor

LGTM on my side

@erwinmombay
Copy link
Member

would love to try and use this on ampproject.org 💃

@cramforce
Copy link
Member Author

Renamed to amp-install-serviceworker

@cramforce cramforce changed the title Implement amp-serviceworker-installation. Implement amp-install-serviceworker Nov 20, 2015
Allows AMP pages to install a ServiceWorker to among other things
implement the pattern described in
https://medium.com/@cramforce/amps-and-websites-in-the-age-of-the-service-worker-8369841dc962
cramforce added a commit that referenced this pull request Nov 20, 2015
Implement amp-install-serviceworker
@cramforce cramforce merged commit fe1a2dd into ampproject:master Nov 20, 2015
@cramforce cramforce deleted the sw-installation branch November 20, 2015 02:11
@jmadler
Copy link
Contributor

jmadler commented Nov 20, 2015

Thanks - spun off #981 for cross-origin support

@dkolba
Copy link

dkolba commented Feb 18, 2016

Does this support "app install banners"?

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.

5 participants