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(manifest): Improvements to scope page #36137

Merged
merged 9 commits into from
Oct 31, 2024

Conversation

dipikabh
Copy link
Contributor

@dipikabh dipikabh commented Oct 1, 2024

Description

This PR is part of improving the web/manifest docs:

  • Adds "Syntax", "Values", and "Description"
  • Covers notes and examples from the spec
  • Adds prose to the examples

Motivation

To ensure all sections have sufficient explanation, all caveats from spec are covered, and the manifest pages follow a similar template

Additional details

Spec links:

Related issues and pull requests

Tracking issue: mdn/mdn#560

Also:
Fixes #32948

@dipikabh dipikabh requested a review from a team as a code owner October 1, 2024 02:21
@dipikabh dipikabh requested review from hamishwillee and removed request for a team October 1, 2024 02:21
@github-actions github-actions bot added Content:Manifest size/m [PR only] 51-500 LoC changed labels Oct 1, 2024
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Preview URLs

(comment last updated: 2024-10-29 22:19:01)

@dipikabh
Copy link
Contributor Author

Thanks for the review, Hamish.

@dipikabh dipikabh requested a review from hamishwillee October 16, 2024 02:56
@dipikabh
Copy link
Contributor Author

Hi Hamish, thanks a lot for the review!

I've done a couple of things in this iteration:

  • Clarified in the intro and in other sections that the behavior is applicable and visible with the installed app (not when using the app in a browser).
  • Moved "In-scope and out-of-scope behavior" to make it the first subsection in "Description" since it explains the core functionality.
    • I've added a web app example and screenshots that hopefully better illustrate the in-scope and out-of-scope behaviors.
  • Added a separate subsection for "Scope's affect on deep-linked pages" and reworded prose to improve clarity

I'm hoping these updates will address your comments. Let me know what you think.

@dipikabh dipikabh requested a review from hamishwillee October 27, 2024 22:30
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Changes look really good. I've added some very minor suggestions.

@dipikabh
Copy link
Contributor Author

Thanks, I've incorporated your suggestions.

@dipikabh dipikabh requested a review from hamishwillee October 29, 2024 22:21
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks very much! Looking good.

@hamishwillee hamishwillee merged commit 831041c into mdn:main Oct 31, 2024
8 checks passed
@dipikabh dipikabh deleted the manifest-scope branch November 1, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Manifest size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Manifest::scope documentation incorrectly states that out-of-scope navigation happens in new browser context
2 participants