Skip to content

Conversation

@danirabbit
Copy link
Member

@danirabbit danirabbit commented Jul 11, 2025

Replaces the "preventer" stack-based width with Granite.MessageDialog

  • Change Kill to Force Quit to avoid violent language
  • Text buttons shouldn't also have text tooltips
  • Only one of Suggested or Destructive should be used for a selection of buttons
  • Make buttons homogeneous
  • Use the extra room we have in Message Dialog to further explain options and their consequences. Use icons to re-enforce outcomes
  • Use action names for button labels instead of ambiguous "yes" and "no"
  • Use ellipsis to indicate that actions aren't triggered immediately but need further action somewhere else
Screenshot from 2025-07-11 10 26 24 Screenshot from 2025-07-11 10 26 30 Screenshot from 2025-07-11 10 30 38

@danirabbit danirabbit requested a review from a team July 11, 2025 17:11
@danirabbit

This comment was marked as resolved.

@danirabbit danirabbit requested a review from a team July 11, 2025 17:17
@stsdc
Copy link
Member

stsdc commented Jul 11, 2025

Agree on everything except the replacement "preventer" with a Granite.MessageDialog.
Do you consider MessageDialog a better UX?

@danirabbit
Copy link
Member Author

@stsdc yes 100%. If the action is something that needs confirmation then messagedialog is the standard way to do that and it needs to have more explanation of why you're requesting to confirm


confirmation_dialog.response.connect ((response) => {
if (response == Gtk.ResponseType.ACCEPT) {
// TODO: maybe add a toast that process killed
Copy link
Member

Choose a reason for hiding this comment

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

I'm inclined toward adding a toast or notification before merging, instead of leaving it to another request. For Shutdown, it's important that the user knows whether or not the request was successfully fulfilled, or if they need to think about Force Quitting instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was already a leftover TODO from main. I'm not adding any functionality here. I'd prefer to leave that to a follow up since I didn't introduce that problem in this branch

@stsdc
Copy link
Member

stsdc commented Jul 11, 2025

@danirabbit OK. But I would apply this change to gtk4-port branch.

@danirabbit
Copy link
Member Author

@stsdc I would prefer to get it in first so we don't have to port the Preventer to Gtk4 and create a bunch of conflicts since you can't subclass Stack in GTK4 😅

@stsdc
Copy link
Member

stsdc commented Jul 11, 2025

ah, ok. Didn't know about this limitation. I will properly review tomorrow and align gtk4 branch accordingly. Thanks @danirabbit!

@danirabbit
Copy link
Member Author

@stsdc haha yeah that was my motivation is I was going to make a GTK4 prep branch for Preventer that doesn't subclass Stack and then I was like "Wait a minute, this should be messagedialog anyways"

Copy link
Member

@wpkelso wpkelso left a comment

Choose a reason for hiding this comment

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

Looks good to me for UX

@stsdc stsdc merged commit c2078e9 into main Jul 12, 2025
4 checks passed
@stsdc stsdc deleted the danirabbit/processinfoview-confirmdialog branch July 12, 2025 12:46
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.

4 participants