! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Center action buttons in conflict dialog window
Status:
RESOLVED: FIXED

Comments

Description fuank 2019-09-18 17:37:27 CEST
Created attachment 9036 
thunar conflict dialog windows are not aligned

When copying / moving multiple files with variable filename lengths, whenever a duplicate conflict arises, the dialog popup window is resized to accommodate the filename's length.

The problem is two folds: 
* The action button widget moves around (it is right-justified, thus sticks to the right corner of the window)
* The window geometry changes if the filename's length is long and wouldn't fit in the window
It is then very easy to miss the "skip" button and click the "replace all" button instead, which leads to potential data loss!

A simple fix would be to make the action buttons centered. 

That way, even if the window gets resized, as long as it is centered by the window manager (which I assume is the case in most cases), the mouse cursor won't risk missing the appropriate action button by having to move around every time.
Comment 1 alexxcons editbugs 2019-09-18 23:16:57 CEST
I agree that it would help to have the buttons centered .. though I dont know how to do it, since a predefined "gtk_dialog_new_with_buttons" is used.
Probably there is a way to get it done. Please feel free to further investigate. Patches are welcome !
thunar_dialogs_show_job_ask_replace is in thunar-dialogs.c:513
Comment 2 fuank 2019-09-19 01:46:27 CEST
Thanks, yes I've been working all day on a patch. So far, I got the buttons to align as wished, but I need to figure out how to implement the callbacks (?) for the buttons. See video attached for demonstration.

Here is part of the code so far (without the pointer declarations), I'll keep hammering on it.

  dialog = gtk_dialog_new();
  gtk_window_set_destroy_with_parent (GTK_WINDOW(dialog), TRUE);
  gtk_window_set_modal (GTK_WINDOW(dialog), TRUE);
  gtk_window_set_title (GTK_WINDOW(dialog), _("Confirm to replace files"));

  gtk_dialog_set_default_response (GTK_DIALOG (dialog), THUNAR_JOB_RESPONSE_YES);
  content_area = gtk_dialog_get_content_area (GTK_DIALOG (dialog));

  button_box = gtk_button_box_new (GTK_ORIENTATION_HORIZONTAL);

  // we're missing the actual responses here!
  cancel_button = gtk_button_new_with_mnemonic (_("_Cancel"));
  skipall_button = gtk_button_new_with_mnemonic (_("S_kip All"));
  skip_button = gtk_button_new_with_mnemonic (_("_Skip"));
  replaceall_button = gtk_button_new_with_mnemonic (_("Replace _All"));
  replace_button = gtk_button_new_with_mnemonic (_("_Replace"));

  gtk_container_add (GTK_CONTAINER(button_box), cancel_button); 
  gtk_container_add (GTK_CONTAINER(button_box), skipall_button); 
  gtk_container_add (GTK_CONTAINER(button_box), skip_button); 
  gtk_container_add (GTK_CONTAINER(button_box), replaceall_button); 
  gtk_container_add (GTK_CONTAINER(button_box), replace_button); 
  gtk_container_add (GTK_CONTAINER(content_area), button_box); 
  gtk_widget_set_hexpand (button_box, GTK_ALIGN_CENTER); 
  gtk_widget_set_halign (button_box, GTK_ALIGN_CENTER);
  gtk_box_pack_start (GTK_BOX (gtk_dialog_get_content_area (GTK_DIALOG (dialog))), button_box, TRUE, FALSE, 0);
  gtk_widget_show (cancel_button); 
  gtk_widget_show (skipall_button); 
  gtk_widget_show (skip_button); 
  gtk_widget_show (replaceall_button); 
  gtk_widget_show (replace_button); 
  gtk_widget_show (button_box);
Comment 4 fuank 2019-09-19 04:46:00 CEST
I got it to work!  It's probably not the best looking code though, but it works. I'll finalize the patch submission soon but feel free to scold me in the meantime if you have suggestions. I'm not used to GTK and C programming. ;)

Not fond of having multiple callbacks instead of just one, but I couldn't figure out how to manipulate the GtkWidget button objects to extract their ID or something.


static void cancel_button_callback ( GtkWidget *button, gpointer dialog)
{
  gtk_dialog_response (GTK_DIALOG(dialog), GTK_RESPONSE_CANCEL);
}
static void skipall_button_callback ( GtkWidget *button, gpointer dialog)
{
  gtk_dialog_response (GTK_DIALOG(dialog), THUNAR_JOB_RESPONSE_NO_ALL);
}
static void skip_button_callback ( GtkWidget *button, gpointer dialog)
{
  gtk_dialog_response (GTK_DIALOG(dialog), THUNAR_JOB_RESPONSE_NO);
}
static void replaceall_button_callback ( GtkWidget *button, gpointer dialog)
{
  gtk_dialog_response (GTK_DIALOG(dialog), THUNAR_JOB_RESPONSE_YES_ALL);
}
static void replace_button_callback ( GtkWidget *button, gpointer dialog)
{
  gtk_dialog_response (GTK_DIALOG(dialog), THUNAR_JOB_RESPONSE_YES);
}

(...)

  GtkWidget         *cancel_button;
  GtkWidget         *button_box;
  GtkWidget         *skipall_button;
  GtkWidget         *skip_button;
  GtkWidget         *replaceall_button;
  GtkWidget         *replace_button;
  GtkWidget         *button_container;

  dialog = gtk_dialog_new();
  gtk_window_set_destroy_with_parent (GTK_WINDOW(dialog), TRUE);
  gtk_window_set_modal (GTK_WINDOW(dialog), TRUE);
  gtk_window_set_title (GTK_WINDOW(dialog), _("Confirm to replace files"));
  gtk_dialog_set_default_response (GTK_DIALOG (dialog), THUNAR_JOB_RESPONSE_YES);

  button_box = gtk_button_box_new (GTK_ORIENTATION_HORIZONTAL);

  cancel_button = gtk_button_new_with_mnemonic (_("_Cancel"));
  skipall_button = gtk_button_new_with_mnemonic (_("S_kip All"));
  skip_button = gtk_button_new_with_mnemonic (_("_Skip"));
  replaceall_button = gtk_button_new_with_mnemonic (_("Replace _All"));
  replace_button = gtk_button_new_with_mnemonic (_("_Replace"));

  gtk_widget_show (cancel_button);
  gtk_widget_show (skipall_button);
  gtk_widget_show (skip_button);
  gtk_widget_show (replaceall_button);
  gtk_widget_show (replace_button);

  g_signal_connect (cancel_button, "clicked", G_CALLBACK (cancel_button_callback), dialog);
  g_signal_connect (skipall_button, "clicked", G_CALLBACK (skipall_button_callback), dialog);
  g_signal_connect (skip_button, "clicked", G_CALLBACK (skip_button_callback), dialog);
  g_signal_connect (replaceall_button, "clicked", G_CALLBACK (replaceall_button_callback), dialog);
  g_signal_connect (replace_button, "clicked", G_CALLBACK (replace_button_callback), dialog);

  gtk_container_add (GTK_CONTAINER(button_box), cancel_button);
  gtk_container_add (GTK_CONTAINER(button_box), skipall_button);
  gtk_container_add (GTK_CONTAINER(button_box), skip_button);  
  gtk_container_add (GTK_CONTAINER(button_box), replaceall_button);
  gtk_container_add (GTK_CONTAINER(button_box), replace_button);
  gtk_container_add (GTK_CONTAINER(content_area), button_box);
  gtk_widget_set_hexpand (button_box, GTK_ALIGN_CENTER); 
  gtk_widget_set_halign (button_box, GTK_ALIGN_CENTER); 

  gtk_widget_show (button_box);

(...)

  /* run the dialog */
  response = gtk_dialog_run (GTK_DIALOG (dialog));
  gtk_widget_destroy (dialog);


Demonstration video:  https://giant.gfycat.com/ColossalTimelyIrishwaterspaniel.mp4  or https://giant.gfycat.com/ColossalTimelyIrishwaterspaniel.webm
Comment 5 fuank 2019-09-19 16:22:18 CEST
Created attachment 9038 
patch to center align buttons in replace dialog

Proposed patch. Create the action area buttons manually, set their callbacks, and align the container widget in the center.
Comment 6 fuank 2019-09-19 17:43:31 CEST
Created attachment 9039 
patch to center align buttons in replace dialog

Forgot to add the transient parent to the dialog. This should be exactly the same behaviour as before now.
Comment 7 fuank 2019-09-19 19:23:01 CEST
Created attachment 9040 
make filename label selectable

Also adding a separate patch to make the conflicting filename label selectable, which is actually super useful. Please consider including it too.
Comment 8 alexxcons editbugs 2019-09-20 10:32:24 CEST
Sorry, currently I am a bit short on time.  Hope I will be able to review/test your patches this weekend.
Comment 9 Git Bot editbugs 2019-09-21 00:13:15 CEST
Alexander Schwinn referenced this bugreport in commit 9ce199d6a0a25d9ad7fdccfa89a6dfe28cbd351f

Center action buttons in conflict dialog window (Bug #15973)

https://git.xfce.org/xfce/thunar/commit?id=9ce199d6a0a25d9ad7fdccfa89a6dfe28cbd351f
Comment 10 Git Bot editbugs 2019-09-21 00:16:32 CEST
Alexander Schwinn referenced this bugreport in commit cc1c4ac50680d22539029506b5e6484cc9afdcba

Center action buttons in conflict dialog window (Bug #15973)

https://git.xfce.org/xfce/thunar/commit?id=cc1c4ac50680d22539029506b5e6484cc9afdcba
Comment 11 alexxcons editbugs 2019-09-21 00:22:45 CEST
Thanks for the patches !

Though I did some search and found this: https://stackoverflow.com/questions/25325609/center-buttons-in-dialog-created-with-gtk-dialog-new-with-buttons/58035619#58035619 
That made the first patch obsolete (sorry, I should have taken a better look before you started writing the patch)
At least now you are a bit more experienced with gtk :)
 
I applied your second patch "as is":
https://git.xfce.org/xfce/thunar/commit/?id=1ad8314689d309afab50f68f3dae9a709cd1c617

I just took you Bugzilla name/mail as author/mail ... hope that was ok .. for the next patch you write, best practice is to use "git format-patch".

I applied both patches on master and the 4.14 branch. Will be released in thunar 1.8.10

Thanks alot for your contribution !
Comment 12 fuank 2019-09-21 01:42:56 CEST
Thanks!

Indeed, but I didn't go that route because gtk_dialog_get_action_area() is now deprecated, so I thought it might be better to just implement the buttons ourselves in order to avoid future headaches. 

Also, the "dialog" GtkWidget should be cast with GTK_DIALOG(dialog) to avoid warnings during compilation.
Comment 13 fuank 2019-09-21 01:51:51 CEST
Actually, I've just tested the patches you've merged in, but it doesn't seem to work?
The buttons are still attached to the right border of the action area.
Comment 14 alexxcons editbugs 2019-09-22 21:24:40 CEST
Crap, you are right, I introduced warnings by that :F  .. I should not rush things.
I will take another look at it.

(In reply to fuank from comment #13)
> Actually, I've just tested the patches you've merged in, but it doesn't seem
> to work?
> The buttons are still attached to the right border of the action area.
That's strange, for me it works fine. Possibly some old Thunar damon was active ? Try "./thunar/thunar -q; ./thunar/thunar" when starting from source code folder to make sure you are using the right one.
Comment 15 fuank 2019-09-22 22:11:24 CEST
> That's strange, for me it works fine. Possibly some old Thunar damon was active ? Try "./thunar/thunar -q; ./thunar/thunar" when starting from source code folder to make sure you are using the right one.

Right, sorry about that, it works fine indeed! Must have been some remnant daemon.
Comment 16 alexxcons editbugs 2019-09-22 23:23:34 CEST
Created attachment 9049 
updated patch

I reworked your patch a bit by using "g_object_set_data" to set a  "response-id" on each button.
Like that a single callback is sufficient.

And I added "gtk_box_set_spacing (GTK_BOX (button_box), 5)" to have similar spacing than before.

The patch is build against the new master (the one which I polluted with my last commit :F )
Would it be ok for you like that ?
Comment 17 fuank 2019-09-23 03:54:47 CEST
Looks good to me! 

That response-id was probably what I was looking for in order to avoid having multiple callbacks. Thanks for fixing it!
Comment 18 Git Bot editbugs 2019-09-23 11:41:31 CEST
fuank referenced this bugreport in commit f709feefc416f9bb09dd885ace351dcbfd6a4167

Center action buttons in conflict dialog window (Bug #15973) - Prevent usage of deprecated gtk_dialog_get_action_area

https://git.xfce.org/xfce/thunar/commit?id=f709feefc416f9bb09dd885ace351dcbfd6a4167
Comment 19 Git Bot editbugs 2019-09-23 11:45:34 CEST
fuank referenced this bugreport in commit bae86115faaae9e2e4b7c28964b0b9864d626010

Center action buttons in conflict dialog window (Bug #15973) - Prevent usage of deprecated gtk_dialog_get_action_area

https://git.xfce.org/xfce/thunar/commit?id=bae86115faaae9e2e4b7c28964b0b9864d626010
Comment 20 alexxcons editbugs 2019-09-23 11:49:08 CEST
Ok, thanks for testing ! I just pushed it.

(In reply to fuank from comment #17)
> That response-id was probably what I was looking for in order to avoid
> having multiple callbacks. Thanks for fixing it!

I just named the data item "response-id", since I thought that would be a good name. Any other name as well would do.

Bug #15973

Reported by:
fuank
Reported on: 2019-09-18
Last modified on: 2019-09-23

People

Assignee:
Xfce Bug Triage
CC List:
1 user

Version

Attachments

Additional information