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

API Review: Web Notification #3186

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Make ReportClicked and ReportClosed async
  • Loading branch information
peiche-jessica committed Jan 10, 2023
commit 993fe9f4284011646c0db20f48f74b80ced1942b
24 changes: 18 additions & 6 deletions specs/WebNotification.md
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,18 @@ interface ICoreWebView2UnsignedLongCollection : IUnknown {
HRESULT GetValueAtIndex([in] UINT index, [out, retval] UINT64* value);
}

/// The caller implements this interface to receive the result of reporting a notification clicked.
[uuid(1013C0D5-5F0C-4BF8-BA52-9D76AFA20E83), object, pointer_default(unique)]
interface ICoreWebView2NotificationReportClickedCompletedHandler : IUnknown {
HRESULT Invoke([in] HRESULT errorCode);
}

/// The caller implements this interface to receive the result of reporting a notification closed.
[uuid(D8B63F74-1D78-4B4C-ACA9-7CB63FDFD74C), object, pointer_default(unique)]
interface ICoreWebView2NotificationReportClosedCompletedHandler : IUnknown {
HRESULT Invoke([in] HRESULT errorCode);
}

/// This is the ICoreWebView2Notification that represents a [HTML Notification
/// object](https://developer.mozilla.org/docs/Web/API/Notification).
[uuid(E3F43572-2930-42EB-BD90-CAC16DE6D942), object, pointer_default(unique)]
Expand Down Expand Up @@ -470,7 +482,7 @@ interface ICoreWebView2Notification : IUnknown {
/// You should only run this if you are handling the `NotificationReceived`
/// event. Returns `E_ABORT` if `Handled` is `FALSE` or `ReportShown` has not
/// been run when this is called.
HRESULT ReportClicked();
HRESULT ReportClicked([in] ICoreWebView2NotificationReportClickedCompletedHandler* handler);

/// The host may run this to report the persistent notification has been
/// activated with a given action, and it will cause the
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no sample showing "with an action" vs ReportClicked. Does no-action mean customer clicked the notification body?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add another sample that does persistent notifications and uses ReportClickedWithAction

Expand All @@ -481,7 +493,7 @@ interface ICoreWebView2Notification : IUnknown {
/// is `FALSE` or `ReportShown` has not been run when this is called. Returns
/// `E_INVALIDARG` if an invalid action index is provided. Use `ReportClicked`
/// to activate an non-persistent notification.
HRESULT ReportClickedWithAction([in] UINT actionIndex);
HRESULT ReportClickedWithAction([in] UINT actionIndex, [in] ICoreWebView2NotificationReportClickedCompletedHandler* handler);
Copy link
Contributor

Choose a reason for hiding this comment

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

"WithActionIndex"? Because you don't pass the Action itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please update.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is an arity-overload in the C# but different method name here. Intentional discrepancy?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes due to COM IDL limitations.


/// The host may run this to report the notification was dismissed, and it
/// will cause the
Expand All @@ -491,7 +503,7 @@ interface ICoreWebView2Notification : IUnknown {
/// event for persistent notifications. You should only run this if you are
/// handling the `NotificationReceived` event. Returns `E_ABORT` if `Handled`
/// is `FALSE` or `ReportShown` has not been run when this is called.
HRESULT ReportClosed();
HRESULT ReportClosed([in] ICoreWebView2NotificationReportClosedCompletedHandler* handler);

/// A string representing the body text of the notification.
/// The default value is an empty string.
Expand Down Expand Up @@ -703,9 +715,9 @@ namespace Microsoft.Web.WebView2.Core
event Windows.Foundation.TypedEventHandler<CoreWebView2Notification, Object> CloseRequested;

void ReportShown();
void ReportClicked();
void ReportClicked(UInt32 actionIndex);
void ReportClosed();
Windows.Foundation.IAsyncAction ReportClickedAsync();
Windows.Foundation.IAsyncAction ReportClickedAsync(UInt32 actionIndex);
Windows.Foundation.IAsyncAction ReportClosedAsync();
}
}
```