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

Service worker no longer required, for Install UI to appear #3215

Merged
merged 14 commits into from
Jul 10, 2024

Conversation

mikehoffms
Copy link
Contributor

@mikehoffms mikehoffms commented Jul 3, 2024

Rendered docset for review:

Changes made by this PR:

  • Mentioned faster & reliable eg offline as reasons to have a service worker. Removed "required for the install UI."
  • Global in repo: lowercased "service worker".
  • Various rewrites, mostly around service workers.
  • Made article title singular: "Use a service worker..."
  • Shrank service worker diagram, lowercased w.
  • Updated a redirected link.
  • TOC:
    • Removed PWA "Reference" toc bucket null node; thus expanded How-to.
    • Made most toc titles match top-of-article title. & hub & global landing.

AB#52267798

@mikehoffms mikehoffms added the cat: pwas Progressive Web Apps-related content label Jul 3, 2024
Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

I added a new commit to remove the TODOs, rephrase the parts that still needed to be rephrased, and update the how-to/index tutorial's content and screenshots.

…ices.md

Co-authored-by: Michael Hoffman <v-mihoffman@microsoft.com>
Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

Approving this, but let's do what you suggested:

  • Use the singular service worker where it makes sense.
  • Change the TOC accordingly.
  • Mention faster, reliable, and offline as benefits of the service worker.

Copy link
Contributor

@captainbrosset captainbrosset left a comment

Choose a reason for hiding this comment

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

My last comments on this are minor improvements, so I'm approving this PR.

Copy link

Learn Build status updates of commit 31f9f32:

✅ Validation status: passed

File Status Preview URL Details
microsoft-edge/developer/index.yml ✅Succeeded View
microsoft-edge/devtools-guide-chromium/progressive-web-apps/index.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/service-workers/index.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2020/08/devtools.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2020/11/devtools.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2021/01/devtools.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2021/04/devtools.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2022/02/devtools.md ✅Succeeded View
microsoft-edge/devtools-guide-chromium/whats-new/2022/03/devtools-100.md ✅Succeeded View
microsoft-edge/extensions-chromium/developer-guide/devtools-extension.md ✅Succeeded View
microsoft-edge/extensions-chromium/developer-guide/migrate-your-extension-from-manifest-v2-to-v3.md ✅Succeeded View
microsoft-edge/index.yml ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/background-syncs.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/best-practices.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/debug.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/handle-files.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/handle-protocols.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/handle-urls.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/icon-theme-color.md ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/devtools-cache.png ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/devtools-offline.png ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/devtools-sw-overview.png ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/sample-pwa-app-available-button.png ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/sample-pwa-app-with-frontend-code.png ✅Succeeded View
microsoft-edge/progressive-web-apps-chromium/how-to/index-images/sample-pwa-app.png ✅Succeeded View

This comment lists only the first 25 files in the pull request.
For more details, please refer to the build report.

For any questions, please:

@captainbrosset captainbrosset merged commit 9b5e638 into main Jul 10, 2024
2 checks passed
@captainbrosset captainbrosset deleted the user/mikehoffms/service-worker-optional branch July 10, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat: pwas Progressive Web Apps-related content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants