Skip to content

feat: add support for notifications#85

Merged
spikecurtis merged 1 commit into
mainfrom
spike/windows-notifications
May 2, 2025
Merged

feat: add support for notifications#85
spikecurtis merged 1 commit into
mainfrom
spike/windows-notifications

Conversation

@spikecurtis
Copy link
Copy Markdown
Collaborator

@spikecurtis spikecurtis commented May 1, 2025

Adds support for OS notifications, which I'll use to show errors handling URIs in a subsequent PR.

Screen Recording 2025-05-01 145532.mp4 (uploaded via Graphite)

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@spikecurtis spikecurtis requested a review from deansheather May 1, 2025 13:09
@spikecurtis spikecurtis marked this pull request as ready for review May 1, 2025 13:10
return ValueTask.CompletedTask;
}

public Task ShowErrorNotification(string title, string message)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could just be ShowNotification, the function itself doesn't seem to be only relevant for errors. I imagine we could use it for other notifications like workspace lifetime notifications in the future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I kinda want to keep the information that this is an "error" in case we want to refactor this class to use, say, a dialog pop-up instead, or decide to style the notifications further to make errors stand out from other notifications.

@spikecurtis spikecurtis merged commit 78ff6da into main May 2, 2025
4 checks passed
@spikecurtis spikecurtis deleted the spike/windows-notifications branch May 2, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants