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

Toast: add dismiss signal with dismissal reason #677

Merged
merged 7 commits into from
May 1, 2024

Conversation

danirabbit
Copy link
Member

@danirabbit danirabbit commented Jan 9, 2024

Always send the closed signal when Toasts are closed manually or automatically, except when the default action is triggered. Also, add a close reason in case we want to respond differently based on whether we closed manually, expired, or withdrawn.

Not sure if we need the close reason actually. afaict nothing is connecting to this signal anywhere. The motivation for this PR is that Desktop settings was watching for the toast revealer to be closed elementary/switchboard-plug-pantheon-shell@c031ada. Happy to exclude that if it seems unnecessary

@danirabbit danirabbit requested a review from a team January 9, 2024 23:37
lib/Widgets/Toast.vala Outdated Show resolved Hide resolved
Copy link
Contributor

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

Just a minor query otherwise looks good.

@tintou
Copy link
Member

tintou commented Jan 23, 2024

Note that adding a parameter to the "close" signal is an API break (meaning that all the code using it will break without being recompiled)

@davidmhewitt
Copy link
Member

davidmhewitt commented Jan 23, 2024

Note that adding a parameter to the "close" signal is an API break (meaning that all the code using it will break without being recompiled)

Yeah, in this case, it may be better to add a new signal (vanished maybe), and mark closed as deprecated. Because not only does it break the API contract, it's also a change in behaviour with the closed signal now being fired for other reasons when it wasn't previously.

@danirabbit danirabbit mentioned this pull request Apr 30, 2024
3 tasks
@danirabbit
Copy link
Member Author

Added a new signal dismissed, after https://valadoc.org/libadwaita-1/Adw.Toast.dismissed.html and deprecated the old signal

@danirabbit danirabbit changed the title Toast: always send close and add close reason Toast: add dismiss signal with dismissal reason Apr 30, 2024
@danirabbit danirabbit merged commit 2858234 into main May 1, 2024
5 checks passed
@danirabbit danirabbit deleted the danirabbit/toast-closereason branch May 1, 2024 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants