-
Notifications
You must be signed in to change notification settings - Fork 195
Adds the web install feature #3488
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
base: main
Are you sure you want to change the base?
Conversation
Adds the We Install feature id `web-install`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening this! This ought to fix #3467. Some suggestions for you in line comments.
| @@ -0,0 +1,3 @@ | |||
| name: web-install | |||
| description: "The `navigator.install` API allows an origin to request an application installation on a user's device." | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a bunch of guidelines for these descriptions. The short version here is that I'm suggesting to make this more direct (avoiding "allows" and "origin") and to mention PWAs to help this come up when searched for related features. I'm not saying it has to be exactly this, but something along these lines would be more consistent with other features:
| description: "The `navigator.install` API allows an origin to request an application installation on a user's device." | |
| description: The `navigator.install()` method triggers a request to install a progressive web app. |
| @@ -0,0 +1,3 @@ | |||
| name: web-install | |||
| description: "The `navigator.install` API allows an origin to request an application installation on a user's device." | |||
| spec: https://github.com/w3c/manifest/pull/1175 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs an exception in the defaultAllowlist in scripts/spec.ts, so the tests pass. (There's a bug for this: #3470.)
| @@ -0,0 +1,3 @@ | |||
| name: web-install | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm skeptical "web install" will become a well-known name for this. I think we should use the interface itself:
| name: web-install | |
| name: navigator.install() |
I'd suggest renaming the file to line up with this (navigator-install.yml or just install.yml).
| @@ -0,0 +1,3 @@ | |||
| name: web-install | |||
| description: "The `navigator.install` API allows an origin to request an application installation on a user's device." | |||
| spec: https://github.com/w3c/manifest/pull/1175 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh one more thing:
| spec: https://github.com/w3c/manifest/pull/1175 | |
| group: progressive-web-app | |
| spec: https://github.com/w3c/manifest/pull/1175 |
Adds the We Install feature id
web-install