easyeffects
easyeffects copied to clipboard
wwmm
Preset save/delete should require confirmation
Unlike loading a preset, which happens often, saving/deleting preset is an extraordinary event, with hard-to-recover consequences. It really should not just happen by a single button press, but require a pop-up "sure? N/y"...
Although the chances of a mistake being made with these buttons is low once you know what they do it makes sense to add some kind of confirmation. But considering the problems GTK currently has with popover menus and dialogs I would wait for them to be fixed before putting more dialogs that have to be called by buttons inside popovers. Some people on Xorg already have problems with the dialog window for preset importing.
That being said if libadwaita already has some kind of custom widget that could do this task without the need for a dialog window we could try to do this now.
the chances of a mistake being made with these buttons is low once you know what they do
I'm afraid knowing what it does, in no way prevents accidental mis-click, since it's literally next to the hot Load button...
I'm afraid knowing what it does, in no way prevents accidental mis-click, since it's literally next to the hot Load button...
It does not prevent an accidental click. But considering the difference in size between these buttons in most cases people will click on the delete button by accident when they do not know yet that this is what the button does. Specially when most translations will make the load button even bigger.
Of course how low this chance is will depend on each person. I can only talk about myself. In all these years I have never clicked in these buttons by accident.
In any case I am not against the idea. I just consider a bad moment to implement it. At least with dialogs. This is the kind of GTK bug that even if I tried I doubt I would be able to send patches to them fixing the current widget focus problems.
That being said if libadwaita already has some kind of custom widget that could do this task without the need for a dialog window we could try to do this now.
There is AdwMessageDialog in libadwaita 1.2 (unreleased), but it doesn't seem very different from the gtk message dialog.
There is AdwMessageDialog in libadwaita 1.2 (unreleased), but it doesn't seem very different from the gtk message dialog.
If we are lucky by the time libadwaita 1.2 is released some of these GTK bugs will be fixed. Or maybe they won't happen with the custom libadwaita dialog.
This bug is super frustrating. I really don't see why on earth this needs support on gtk/libadwaita side though.
I really don't see why on earth this needs support on gtk/libadwaita side though.
It is not about needing support in adwaita but about GTK menus and dialogs not playing nice with each other in some users setups. Like the ones where people use xorg instead of Wayland. That is why I would like to avoid showing a dialog window there...
We already use the AdwMessageDialog for corrupted presets not correctly loaded, but it's a simple dialog with the close button. We added (press ESC to close) because of that bug. But for this case we would need a new dialog with Yes-No buttons handling with callables.
It's quite cumbersome for us to deploy it and maybe that bug will also prevent the correct execution. Besided it could be also cumbersome for some users that want to delete multiple presets and they have to click Yes every time on the dialog.
The best solution in my opinion is to separate the delete button from the saving button and maybe applying it a class to show it as red (libAdwaita has something to do that, but I don't remember the specific feature).
So there is no longer a save button? How would one save the preset then?
The save button is still where it was.
Besides save and delete buttons have tooltips, so going with the pointer on them the user has suggestions on their meaning.
Thanks for taking a look!
The save button is still where it was.
Honestly, i did not even notice that those were two buttons, and not one big "load" button.
I understand that they have cool looks, names and tooltips, but what i'm looking for, is idiot-proofing. None of that shiny UX helps with misclicking.
@LebedevRI Well, at least now you can't misclick the delete button since it's separated.
It's not that it's cool. Libadwaita has this class named destructive-action and I though it was a good idea to use on something that deletes the preset.
wwmm @LebedevRI what do you think?
I think it is fine.
Honestly, i did not even notice that those were two buttons, and not one big "load" button.
Ideally I would like to not have a "load" button and have the click on the preset row as way to activate the preset. But I am not sure if people will consider this confusing.
Today while using chatgpt on my phone web browser I've payed attention to how they are handling the history entries removal confirmation. In the list they have for each entry a delete button similar to ours. When we click on it the whole line is replaced by another one where there is a text asking if you want to delete and 2 new buttons. One for confirmation and another to cancel.
I think this is something doable on EasyEffects. We could for example use an overlay that would be shown only over the row where the button was clicked. This way we still can click on the buttons of the other rows as usual and deleting multiple presets would not be so annoying as it would with a dialog window hiding everything. And we would also avoid the current gtk4 bugs related to dialog windows. What do you think @Digitalone1 and @LebedevRI ?
It's a nice idea but I wonder what to use with the overlay. I don't remember very well how to use it, but it should be something different from the toasts because toasts do not have full opacity and you can see behind.
It's a nice idea but I wonder what to use with the overlay. I don't remember very well how to use it, but it should be something different from the toasts because toasts do not have full opacity and you can see behind.
Looking at how things are done in https://github.com/wwmm/easyeffects/blob/master/data/ui/apps_box.ui I think that inside <child type="overlay"> we can put a GtkBox with a label and the 2 confirmation buttons. And in the same place where <object class="GtkScrolledWindow"> is we could put the current row widgets. I think it will work. I will do some tests in the next days.
So far the easiest workaround is hiding the row widgets that were being shown in the background through
<property name="visible" bind-source="overlay" bind-property="visible" bind-flags="sync-create|invert-boolean" />
What do you think @Digitalone1 and @LebedevRI ?
I think that would be an improvement in the right direction, thank you for looking into it!
I have updated the master branch with the new approach to preset saving/removal. Now it has to be tested and improved where necessary. This is an example image
With the confirmation menu I think there is no need for the delete button to be so "ominous". So I brought it back to how it was before.
It's nice, but I think the question should have a different styling because it can be confused with the other presets in the list. It's better to highlight them somehow.
Look here: https://gnome.pages.gitlab.gnome.org/libadwaita/doc/1-latest/named-colors.html
Maybe accent for both save and delete. Or warning for save and destructive for delete?
but I think the question should have a different styling because it can be confused with the other presets in the list.
Yes. After some time I thought the same thing.
Maybe accent for both save and delete. Or warning for save and destructive for delete?
Definitely destructive for delete. I am not sure about the other yet.
For some reason the text in the gtklabel does not get red when using the destructive-action class. Maybe it works only for buttons.
This is how it looks with the error and warning classes
I have updated the master branch with this.
@wwmm I think it's good to make another minor release. Not only to push this, but also to fix the reset issue.
I think it's good to make another minor release. Not only to push this, but also to fix the reset issue.
Ok. Do you want some time to translate the new strings?