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

Add Close and ClearAll api for INotificationManager and IManagedNotificationManager #15628

Merged
merged 10 commits into from
May 7, 2024

Conversation

wieslawsoltes
Copy link
Collaborator

@wieslawsoltes wieslawsoltes commented May 6, 2024

What does the pull request do?

  • Adds Close() and ClearAll() methods to INotificationManager/IManagedNotificationManager
  • Adds CancellationToken parameter to Show method for INotificationManager/IManagedNotificationManager

What is the current behavior?

There is no easy way to close notification from code.

What is the updated/expected behavior with this PR?

There is easy way to close notification from code.

How was the solution implemented (if it's not obvious)?

Adds Close() and ClearAll() methods to INotificationManager/IManagedNotificationManager

Checklist

Breaking changes

  • Added CancellationToken parameter INotificationManager.Show method
  • Added INotificationManager.Close method
  • Added INotificationManager.ClearAll method
  • Added CancellationToken parameter IManagedNotificationManager.Show method
  • Added IManagedNotificationManager.Close method

Obsoletions / Deprecations

Fixed issues

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0048149-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@cla-avalonia
Copy link
Collaborator

cla-avalonia commented May 7, 2024

  • All contributors have signed the CLA.

@wieslawsoltes
Copy link
Collaborator Author

@cla-avalonia agree

@@ -13,6 +14,18 @@ public interface INotificationManager
/// Show a notification.
/// </summary>
/// <param name="notification">The notification to be displayed.</param>
void Show(INotification notification);
/// <param name="cancellationToken">The cancellation token.</param>
void Show(INotification notification, CancellationToken? cancellationToken = null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why CancellationToken? cancellationToken = null
CancellationToken cancellationToken = default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@maxkatz6
Copy link
Member

maxkatz6 commented May 7, 2024

It will likely go into 11.1.1

@wieslawsoltes
Copy link
Collaborator Author

It will likely go into 11.1.1

Can x.x.1 have such breaking api changes?

@maxkatz6
Copy link
Member

maxkatz6 commented May 7, 2024

@wieslawsoltes in current state of this PR, new interface members are added on NonClientImplementable interface. No user should be able to implement this interface either way.

CancellationToken parameter could break some existing apps, but it was reverted, so it's fine.

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0048155-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@jmacato jmacato added this pull request to the merge queue May 7, 2024
@workgroupengineering
Copy link
Contributor

@wieslawsoltes in current state of this PR, new interface members are added on NonClientImplementable interface. No user should be able to implement this interface either way.

CancellationToken parameter could break some existing apps, but it was reverted, so it's fine.

instead of resetting you can add an show overload. The CancellationToken parameter is very useful.

Merged via the queue into AvaloniaUI:master with commit f7feaba May 7, 2024
10 checks passed
@wieslawsoltes wieslawsoltes deleted the ExtendNotificationManager branch May 7, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants