-
Notifications
You must be signed in to change notification settings - Fork 43
Add app badge to Declarative Web Push #402
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: gh-pages
Are you sure you want to change the base?
Conversation
</li> | ||
<li> | ||
<p> | ||
[=Set the application badge for installed web application=] given |
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.
See w3c/badging#116
@annevk, to kick this off:
happy to refine from here... but it's broadly what needs to happen, right? |
index.html
Outdated
</dt> | ||
<dd> | ||
<p> | ||
A [=/64-bit unsigned integer=]. |
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 noting that not all platforms support setting a number: for instance, Android allows setting the badge to "flag" (which just shows up as a dot). Some macOS apps do this too (e.g., Slack).
For backwards compat, we might need to default change the badging API to support an enum to explicitly set a "flag"
as the default value, so that:
- setAppBadge() - "flag"
- setAppBadge(0) - clear
- setAppBadge(N) - number.
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.
So you're saying that even platforms that do render a number, can also render just the badge without the number? In that case we'd indeed need to support that separate state. We could perhaps allow passing undefined or null for that? Bit finicky, I'll admit.
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.
So you're saying that even platforms that do render a number, can also render just the badge without the number?
Correct. Android in particular supports both modes ("App Icon can Show Badges with Numbers or Dot-style Badges"). Even though it's user controlled, a web developer can still just set a "dot" (i.e., what in the Badging spec is called a "flag").
Bit finicky, I'll admit.
Yeah :( hopefully we can do it consistently across both specs.
Possibly the most compatible option would be to change app_badge
to support either a boolean
or a 64-bit unsigned integer
:
- If app_badge is missing: do nothing.
- If app_badge === true: then "flag.
- Else if app_badge === false: "clear" (treat at 0).
- Else if typeof app_badge == "number", set number.
We can distinguish between the types, so could potentially work.
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.
That shape sounds good. What happens for platforms that don't support "flag" though?
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.
Described in w3c/badging#123
registration/has shownotification() been successfully invoked|has | ||
`showNotification()` been successfully invoked=]. | ||
Set |appBadgeResult| to true if {{NavigatorBadge/setAppBadge()}} has been | ||
invoked; otherwise false. |
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.
@marcoscaceres we need a hook for this similar to "has showNotification()
been successfully invoked". People were not happy with this being defined in a hand-wavy way.
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.
IIUC, in the case of notifications, it emulates calling showNotification()
...
Instead, for the Badging case, can we reuse "set the application badge for an installed web application" and I can get that to return a boolean if it worked?
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.
No, this is specifically to catch the case where the developer used setAppBadge()
(and maybe even clearAppBadge()
, come to think of it) and so we don't want to use the declarative app badge result.
See https://notifications.spec.whatwg.org/#service-worker-registration-has-shownotification-been-successfully-invoked and how showNotification()
ends up setting it to true. (We set it to false here first before dispatching the event to the service worker.) We need the same for app badge.
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.
ah, ok... I'll have another look.
<li data-cite="appmanifest"> | ||
<p> | ||
If |appBadgeSet| is false and the web application associated with the <a>push | ||
subscription</a> is an [=installed web application=]: |
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 should use |subscription| here, I think? Maybe
If appBadgeSet is false and |subscription| has an associated installed web application:
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.
Though it would be nice to also support this outside of installed web applications. But I suppose that's a separate change.
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.
Setting a badge is pretty tightly bound to installation at the moment, as that's how it's been implemented by everyone. We could change this if this changes.
To land this w3c/badging#111 needs to be completed. I'd like some help with that. @marcoscaceres maybe?
The following tasks have been completed:
Implementation support:
Preview | Diff