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.

Sign up for GitHub

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

Jump to bottom

Add support for inline replies #132

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add support for inline replies #132

Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
904bcad
Adding Inline Notification Replies to the spec
May 22, 2018
eac4ebe
Updating based on annevk's review
May 24, 2018
844b530
address some nits
annevk May 28, 2018
1dba647
s/A action/An action
May 29, 2018
e308162
Addressing (some of) Peter's comments
May 31, 2018
8467059
Update algorithm in 'Closing a notification' to pass action and reply
Jun 6, 2018
56bb9a1
Add a note encouraging attribution
Jun 22, 2018
b8426e9
Change note to mandate attribution
anitawoodruff Jul 10, 2018
301ccf8
Amend note on treating text actions as buttons
anitawoodruff Jul 13, 2018
713eafb
Add note on destination of reply data
anitawoodruff Jul 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 85 additions & 23 deletions notifications.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,42 @@ the <a>notification</a>, but then it should have less visual priority than the
is not otherwise accessible to the end user, especially since notification platforms that do not
support these features might ignore them.

<p>A <a>notification</a> has an associated list of
zero or more <dfn for=notification lt=action id=actions>actions</dfn>. Each
<a for=notification>action</a> has an associated
<dfn for=action id=action-title>title</dfn> and <dfn for=action id=action-name>name</dfn> and
<em>can</em> have an associated <dfn for=action>icon URL</dfn> and
<dfn for=action>icon resource</dfn>. Users may activate actions, as alternatives to
activating the notification itself. The user agent must determine the
<dfn>maximum number of actions</dfn> supported, within the constraints of the
notification platform.
<p>A <a>notification</a> has an action list (a <a for=/>list</a> of
Copy link
Member

Choose a reason for hiding this comment

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

"action list" needs to be a <dfn> (and some places that currently refer to "actions" need to be updated accordingly)

<dfn for=notification lt=action id=actions>actions</dfn>). It is initially empty.
Copy link
Member

Choose a reason for hiding this comment

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

This should become a reference <a for=notification>action</a> and then this <dfn> can replace the <dfn> below so we just have "action" and not "notification action".


<p>A <dfn id=notification-action>notification action</dfn> defines a way the user can interact with
a notification, as an alternative to activing the notification itself.

<div dfn-for="notification action">
<p>A <a>notification action</a> has a <dfn>type</dfn>, that is "<code>button</code>" or
"<code>text</code>".

<p class=note>Actions of type "<code>button</code>" can only be activated, whereas actions of type
"<code>text</code>" allow the user to input text during activation.

<p>A <a>notification action</a> has a <dfn id=action-title>title</dfn> (a string).

<p>A <a>notification action</a> has a <dfn id=action-name>name</dfn> (a string).

<p>A <a>notification action</a> has an <dfn>icon URL</dfn> (null or a <a>URL</a>). Unless stated
otherwise, it is null.

<p>A <a>notification action</a> has an <dfn>icon resource</dfn> (null or an image). Unless stated
otherwise, it is null.

<p>A <a>notification action</a> has a <dfn>placeholder</dfn> (a string). Unless stated otherwise,
it is an empty string.
</div>

<p>The user agent must determine the <dfn>maximum number of actions</dfn> supported, within the
constraints of the notification platform.

<p class=note>Since display of actions is platform-dependent, developers are
encouraged to make sure that any action a user can invoke from a notification is
also available within the web application.
also available within the web application. The ability to reply inline to
notifications during activation is also platform-dependent, so developers are encouraged to
handle the case where a text action was activated but the reply was null (e.g., by focusing a chat
window).

<p class="note no-backref">Some platforms might modify an <a for=action>icon resource</a> to better
match the platform's visual style before displaying it to the user, for example by rounding the
Expand Down Expand Up @@ -227,14 +250,22 @@ these steps:
<li><p>If <var>options</var>'s <code>requireInteraction</code> is true, set
<var>notification</var>'s <a for=notification>require interaction preference flag</a>.

<li><p>Set <var>notification</var>'s list of <a>actions</a> to an empty list,
then for each <var>entry</var> in <var>options</var>'s <code>actions</code>,
up to the <a>maximum number of actions</a> supported (skip any excess
entries), perform the following steps:
<li><p>For each <var>entry</var> in <var>options</var>'s <code>actions</code>:

<ol>
<li><p>If the user agent does not support additional actions, then break.

<li><p>Let <var>action</var> be a new <a lt="actions">action</a>.

<li><p>Set <var>action</var>'s <a for=action>type</a> to the <var>entry</var>'s
<code>type</code> property.

<li><p>If the user agent does not support additional actions of this
<a for="notification action">type</a>, then continue.

<p class=note>For instance, some operating systems don't support buttons if a text action has
been provided, or vice-versa.

<li><p>Set <var>action</var>'s <a for=action>name</a> to the <var>entry</var>'s
<code>action</code>.

Expand All @@ -244,7 +275,11 @@ these steps:
<li><p>If <var>entry</var>'s <code>icon</code> is present,
<a lt="url parser">parse</a> it using <var>baseURL</var>, and if that does
not return failure, set <var>action</var>'s <a for=action>icon URL</a> to the
return value. (Otherwise <a for=action>icon URL</a> is not set.)
return value.

<li><p>If <var>entry</var>'s <code>placeholder</code> is present, set
action's <a for=action>placeholder</a> to the <var>entry</var>'s
<code>placeholder</code>.

<li><p>Append <var>action</var> to <var>notification</var>'s list of
<a>actions</a>.
Expand Down Expand Up @@ -517,15 +552,22 @@ specified) run these steps:
<p>If <var>notification</var> is a <a>persistent notification</a>, then:

<ol>
<li><p>Let <var>action</var> be the empty string.
<li><p>Let <var>action</var> be an empty string.

<li><p>Let <var>reply</var> be null.

<li><p>If one of <var>notification</var>'s <a for=notification>actions</a> was activated by the
user, then set <var>action</var> to that <a for=notification>action</a>'s <a for=action>name</a>.

<li><p>If one of <var>notification</var>'s <a for=notification>actions</a>
with type <code>text</code> was activated by the user, and the opportunity to
input a reply was provided, then set <var>reply</var> to the text entered by
the user during activation.

<li><p>Let <var>callback</var> be an algorithm that when invoked with a <var>global</var>,
<a lt="fire a service worker notification event named e">fires a service worker notification event</a>
named <code>notificationclick</code> given <var>notification</var> and <var>action</var> on
<var>global</var>.
named <code>notificationclick</code> given <var>notification</var>, <var>action</var> and
<var>reply</var> on <var>global</var>.

<li><p>Then run <a>Handle Functional Event</a> with <var>notification</var>'s
<a for=notification>service worker registration</a> and <var>callback</var>.
Expand Down Expand Up @@ -666,9 +708,16 @@ enum NotificationDirection {
};

dictionary NotificationAction {
NotificationActionType type = "button";
required DOMString action;
required DOMString title;
USVString icon;
DOMString? placeholder = "";
};

enum NotificationActionType {
"button",
"text"
};

callback NotificationPermissionCallback = void (NotificationPermission permission);
Expand Down Expand Up @@ -863,12 +912,14 @@ result of the following steps:
<ol>
<li><p>Let <var>frozenActions</var> be an empty list of type {{NotificationAction}}.

<li><p>For each <var>entry</var> in the <a>notification</a>'s list of
<a for=notification>actions</a>, perform the following steps:
<li><p>For each <var>entry</var> in <a>notification</a>'s <a for=notification>actions</a>:

<ol>
<li><p>Let <var>action</var> be a new {{NotificationAction}}.

<li><p>Set <var>action</var>'s {{NotificationAction/type}} to
<var>entry</var>'s <a for=action>type</a>.

<li><p>Set <var>action</var>'s {{NotificationAction/action}} to
<var>entry</var>'s <a for=action>name</a>.

Expand All @@ -878,6 +929,9 @@ result of the following steps:
<li><p>Set <var>action</var>'s {{NotificationAction/icon}} to
<var>entry</var>'s <a for=action>icon URL</a>.

<li><p>Set <var>action</var>'s {{NotificationAction/placeholder}} to
<var>entry</var>'s <a for=action>placeholder</a>.

<!-- XXX IDL dictionaries are usually returned by value, so don't need to be
immutable. But FrozenArray reifies the dictionaries to mutable JS
objects accessed by reference, so we explicitly freeze them.
Expand Down Expand Up @@ -1000,11 +1054,13 @@ partial interface ServiceWorkerRegistration {
interface NotificationEvent : ExtendableEvent {
readonly attribute Notification notification;
readonly attribute DOMString action;
readonly attribute DOMString? reply;
};

dictionary NotificationEventInit : ExtendableEventInit {
required Notification notification;
DOMString action = "";
DOMString? reply = null;
};

partial interface ServiceWorkerGlobalScope {
Expand Down Expand Up @@ -1084,11 +1140,13 @@ the same underlying <a>notification</a> of {{Notification}} objects already in e
<hr>

<p>To <dfn>fire a service worker notification event named <var>e</var></dfn>
given <var>notification</var> and <var>action</var>,
given <var>notification</var>, <var>action</var> and <var>reply</var>,
<a>fire an event</a> named <var>e</var>, using {{NotificationEvent}}, with the
{{NotificationEvent/notification}} attribute initialized to a new
{{Notification}} object representing <var>notification</var> and the
{{NotificationEvent/action}} attribute initialized to <var>action</var>.
{{Notification}} object representing <var>notification</var>, the
{{NotificationEvent/action}} attribute initialized to <var>action</var>, and the
{{NotificationEvent/reply}} attribute initialized to <var>reply</var>.
Copy link
Member

Choose a reason for hiding this comment

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

Where does the reply variable come from?

Copy link
Author

Choose a reason for hiding this comment

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

Ah good point, guess I can just change the beginning of this sentence to 'given notification, action and reply' which should cover it, yes?

Copy link
Member

Choose a reason for hiding this comment

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

Well, you'll need to update the callers of this algorithm too, then.

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, that makes sense.

Have updated Step 1.5 of Section 2.7 'Activating a Notification' to 'given notification, action and reply' also. I think this now covers it.

Copy link
Member

Choose a reason for hiding this comment

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

There's two callers of this algorithm, so I don't think that quite covers it. Also, if both are going to set it to null, does it need to be a variable for this algorithm?

Copy link
Author

Choose a reason for hiding this comment

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

Step 1.4 of Section 2.7 ('Activating a notification') sets 'reply' to the text entered by the user during activation, so it shouldn't be null in that case.

I assume you are talking about Section 2.8 ('Closing a notification') as the other caller of this algorithm. That caller does not pass anything for 'action' either right now, so I guess this needs fixing too?

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah :/ I guess action needs to be the empty string there; @beverloo can confirm.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it should be. We could consider changing the event hierarchy a bit to avoid including the action and reply fields with close events - it'd add a bit of boilerplate, but seems cleaner.

NotificationEvent { .notification }
NotificationClickEvent : NotificationEvent { .action, .reply }

Copy link
Author

Choose a reason for hiding this comment

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

Hm yeah that would involve adding a whole new interface to the ServiceWorker API, no?

I'm up for doing this but won't this involve quite a lot of work updating idls and tests etc? Could it be done as a followup in a separate change?

Copy link
Author

Choose a reason for hiding this comment

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

Agreed with beverloo@ offline that changing the event hierarchy is out of scope for this change.

Have now updated the 'Closing a notification' algorithm as suggested to pass the empty string & null.


<!-- XXX need to define at what the event is fired -->

<p>The {{NotificationEvent/notification}} attribute's getter must return the
Expand All @@ -1097,6 +1155,9 @@ value it was initialized to.
<p>The {{NotificationEvent/action}} attribute's getter must return the value it
was initialized to.

<p>The {{NotificationEvent/reply}} attribute's getter must return the value it
was initialized to.

<p>The following is the <a>event handler</a> (and its corresponding
<a>event handler event type</a>) that must be supported as attribute by the
{{ServiceWorkerGlobalScope}} object:
Expand All @@ -1122,6 +1183,7 @@ was initialized to.
Addison Phillips,
Aharon (Vladimir) Lanin,
Alex Russell,
Anita Woodruff,
Anssi Kostiainen,
Arkadiusz Michalski,
Boris Zbarsky,
Expand Down