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

docs(gatsby-plugin-offline): Show how to handle a SW update #10417

Merged
merged 3 commits into from
Dec 11, 2018
Merged

docs(gatsby-plugin-offline): Show how to handle a SW update #10417

merged 3 commits into from
Dec 11, 2018

Conversation

valin4tor
Copy link
Contributor

@valin4tor valin4tor commented Dec 11, 2018

Closes #9087

I also deleted /docs/add-offline-support/ since it just redirects to /docs/add-offline-support-with-a-service-worker/ anyway, and hasn't been updated in a while.

@valin4tor valin4tor requested a review from a team as a code owner December 11, 2018 18:51
@valin4tor valin4tor requested a review from a team December 11, 2018 18:51
Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

Looks great; left a little comment!

@@ -206,6 +206,10 @@ exports.onServiceWorkerInstalled = true
* Inform plugins of when a service worker has an update available.
* @param {object} $0
* @param {object} $0.serviceWorker The service worker instance.
* @example
* exports.onServiceWorkerUpdateFound = () => {
* window.alert(`An updated service worker was found.`)
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally want to keep these examples relatively useful (e.g. users will probably directly copy and paste this).

Is this the best example we could show? I'm almost inclined to suggest reverting it back to the way it was, but minimally I think perhaps a prompt like you've shown earlier is more effective?

Co-Authored-By: davidbailey00 <4248177+davidbailey00@users.noreply.github.com>
Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

Looks great! Happy to see your first docs PR 🙌

@DSchau DSchau merged commit c181e5f into gatsbyjs:master Dec 11, 2018
@shannonbux
Copy link
Contributor

@davidbailey00 looks great! Thanks for noticing the redundancy too; I thought I had fixed that and clearly hadn't! Awesome!

gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
…#10417)

Closes gatsbyjs#9087

I also deleted `/docs/add-offline-support/` since it just redirects to `/docs/add-offline-support-with-a-service-worker/` anyway, and hasn't been updated in a while.
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.

None yet

3 participants