Skip to content

Add closable notifications with dismiss-aware callbacks - #142

Open
corradedied wants to merge 8 commits into
deividcomsono:mainfrom
corradedied:notification-closing
Open

Add closable notifications with dismiss-aware callbacks#142
corradedied wants to merge 8 commits into
deividcomsono:mainfrom
corradedied:notification-closing

Conversation

@corradedied

@corradedied corradedied commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

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 Callback that 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 Notify options

Library:Notify({
    Title = "mspaint",
    Description = "Hello world!",
    Time = 4,
    Closable = true, -- shows the close ("X") button
    Callback = function(Notification, Reason)
        -- Reason: "time" | "manual" | "code"
    end,
})
  • Closable — 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).
  • If a notification is Persist = true and has a Callback, Closable is 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 same Data:Destroy(Reason), tagged with why it happened:

  • Timer/timeout completing → Destroy("time")
  • Close button clicked → Destroy("manual")
  • Called directly with no argument → Destroy("code")

Destroy always fires Callback (if set) and passes Reason along 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

close callback

Example (as in the GIFs above)

GroupBox1:AddButton("sendNotif", function()
	Library:Notify({
		Title = "notification",
		Description = "this is a notification",
		Time = 2,
        Closable = true, -- adds the close ("X") button
		Callback = function(notif, reason)
			warn("fired close callback. \nTitle: " .. notif.Title .. "\nDescription: " .. notif.Description .. "\nReason: " .. reason) -- i know i could just use string.format
		end
	})
end)

@corradedied corradedied changed the title Add closable notifications with dismiss-aware callbacks Fixing conflicts soon - Add closable notifications with dismiss-aware callbacks Aug 20, 2026
@corradedied
corradedied force-pushed the notification-closing branch from 6b2ee25 to 3f86ed8 Compare August 21, 2026 00:15
@corradedied

corradedied commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

resolved sync conflicts

@corradedied corradedied reopened this Aug 21, 2026
@corradedied corradedied changed the title Fixing conflicts soon - Add closable notifications with dismiss-aware callbacks Add closable notifications with dismiss-aware callbacks Aug 21, 2026

@mstudio45 mstudio45 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • CallbackOnAutoDismiss should 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
@corradedied
corradedied requested a review from mstudio45 August 23, 2026 21:45
@corradedied

corradedied commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author
  • CallbackOnAutoDismiss should 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).

I've just pushed a fix regarding both.

  • Removed CallbackOnAutoDismiss entirely. Callback now fires unconditionally on every Destroy(), no matter how it was triggered.
  • Renamed the reasons to "time", "manual", "code" (removed "programmatic").

The PR message has been updated. Let me know if you'd rather want "code" dropped in favor of just two reasons ("code"/"manual") instead of three.

@corradedied corradedied changed the title Add closable notifications with dismiss-aware callbacks Fixing conflicts soon - Add closable notifications with dismiss-aware callbacks Aug 25, 2026
@corradedied corradedied changed the title Fixing conflicts soon - Add closable notifications with dismiss-aware callbacks Add closable notifications with dismiss-aware callbacks Aug 26, 2026
@corradedied
corradedied force-pushed the notification-closing branch from 7c830a4 to ff558ef Compare August 26, 2026 03:15
@corradedied corradedied reopened this Aug 26, 2026
Comment thread Library.lua
end

function Data:Destroy()
function Data:Destroy(Reason: string?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reason: "time" | "manual" | "code"

fuck github desktop for forcing me to force-push 10 times.......
@corradedied

Copy link
Copy Markdown
Contributor Author

resolved conflicts and synced changes

@corradedied corradedied reopened this Aug 26, 2026
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