-
Notifications
You must be signed in to change notification settings - Fork 161
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
Changed to allow site-triggered prompt without defaulting to automatic prompt #584
Conversation
…s to notify that an install prompt is available'.
…ted install prompt.
Explicitly allow the user agent to run steps to notify without being prior to an automated install prompt.
index.html
Outdated
@@ -396,21 +396,36 @@ | |||
Install prompts | |||
</h2> | |||
<p> | |||
An end-user can <dfn data-lt="manual installation">manually</dfn> | |||
There are three ways that the installation process can be triggered: |
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.
maybe change this to "multiple ways" instead of "three"... always forget to update those numbers when new things get added.
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.
🎉 Just a couple of little things. Looking good.
index.html
Outdated
<li>The <a>installation process</a> can occur through a | ||
<dfn>site-triggered install prompt</dfn>: the site can | ||
programmatically request that the user agent present an install | ||
prompt to the user. The availability of this feature may be |
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.
s/The availability of this feature may be/The user agent MAY restrict the availability of this feature/
index.html
Outdated
MUST run the <a>steps to notify before an automated install | ||
prompt</a>. | ||
MUST run the <a>steps to notify that an install prompt is | ||
available</a>, to give the site the opportunity to suppress the |
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 wonder if we should say, instead of "to give the site the opportunity to suppress the install prompt" something like "prevent the default action (which is to install the application)". That would maybe align better with . preventDefault()
index.html
Outdated
prompt</a>. | ||
MUST run the <a>steps to notify that an install prompt is | ||
available</a>, to give the site the opportunity to suppress the | ||
install prompt. Alternatively, the user agent may run the <a>steps to |
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.
s/may/MAY
index.html
Outdated
<div class="note"> | ||
The <code>beforeinstallprompt</code> event is somewhat misnamed, as | ||
it does not necessarily signal that an <a>automated install | ||
prompt</a> will follow (depending on the user agent, it may just be |
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.
s/it may just/it can just
or "it might just"
(avoid RFC2119 keywords in notes... even in lowercase)
index.html
Outdated
discretion (rather than prompting at an arbitrary time), whilst | ||
still providing a prominent UI to do so. | ||
prompt from showing until the user clicks a button to show a | ||
site-triggered install prompt. In this way, the site can leave |
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.
nit: link back to def <a>site-triggered install prompt</a>
@mgiuca, just FYI, we added a magic service that generates previews/diff for us. It adds the links at the top of the Pull Request. (PS: if you want it for Web Share, just copy this file to that repo: https://github.com/w3c/manifest/blob/gh-pages/.pr-preview.json and make any changes needed. Should be straight forward) |
*FYI |
Done all of those. Thanks for the review. Whoa, the pr-preview is magical. Trying it on Web Share. |
Closes #576
Preview | Diff