Add closable notifications with dismiss-aware callbacks - #142
Open
corradedied wants to merge 8 commits into
Open
Add closable notifications with dismiss-aware callbacks#142corradedied wants to merge 8 commits into
corradedied wants to merge 8 commits into
Conversation
corradedied
force-pushed
the
notification-closing
branch
from
August 21, 2026 00:15
6b2ee25 to
3f86ed8
Compare
Contributor
Author
|
resolved sync conflicts |
Remove disconnection logic for PriorityIndicatorConnection.
mstudio45
requested changes
Aug 23, 2026
mstudio45
left a comment
Collaborator
There was a problem hiding this comment.
CallbackOnAutoDismissshould not even exist and Callback should fire no mather what on Destroy if it exists.- The reasons should be
"code" and "manual"or"time", "manual" and "code"but I am still not sure (I don't really like "programmatic" but I don't know what would be better).
- Remove CallbackOnAutoDismiss; Callback now fires on every Destroy - Rename Reason values: auto -> time, programmatic -> code Addresses review from mstudio45 on PR deividcomsono#142
Contributor
Author
I've just pushed a fix regarding both.
The PR message has been updated. Let me know if you'd rather want |
corradedied
force-pushed
the
notification-closing
branch
from
August 26, 2026 03:15
7c830a4 to
ff558ef
Compare
corradedied
commented
Aug 26, 2026
| end | ||
|
|
||
| function Data:Destroy() | ||
| function Data:Destroy(Reason: string?) |
Contributor
Author
There was a problem hiding this comment.
Reason: "time" | "manual" | "code"
corradedied
force-pushed
the
notification-closing
branch
from
August 26, 2026 03:34
4489448 to
7c830a4
Compare
corradedied
force-pushed
the
notification-closing
branch
from
August 26, 2026 03:35
7c830a4 to
ff558ef
Compare
…/Obsidian into notification-closing
fuck github desktop for forcing me to force-push 10 times.......
Contributor
Author
|
resolved conflicts and synced changes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested by FlamesW and based on reply from mstudio45
Summary
Notifications currently have no way to be dismissed early or to run code when they close. This adds an optional "X" close button and a
Callbackthat can tell you why a notification was dismissed, so notifications can be used for things like confirmations, undo prompts, or anything needing a response tied to how the user (or the clock) closed it.New
NotifyoptionsClosable— shows an "X" in the top-right corner using the existing lucide icon implementation (Library:GetIcon("x")), not a unicode glyph.Callback— runs every time the notification is dismissed, regardless of how it was dismissed. Receives(Notification, Reason).Persist = trueand has aCallback,Closableis force-enabled even if not set — otherwise there'd be no way to ever dismiss it and trigger that callback.How it works internally
All dismissal paths (timeout, close-button click, or a script calling
Notification:Destroy()directly) now route through the sameData:Destroy(Reason), tagged with why it happened:Destroy("time")Destroy("manual")Destroy("code")Destroyalways firesCallback(if set) and passesReasonalong as the callback's second argument so calling code can branch on it if it cares. This keeps the firing logic and the actual cleanup (timer disposal, position updates, tween-out) in one place instead of duplicated across each dismissal path.Visuals
Example (as in the GIFs above)