Created attachment 226092 [details] [review]
wizard: Same size for all toolbar buttons

Put toolbar buttons in a sizegroup so they are equally sized.

Created attachment 226094 [details] [review]
wizard: Next button should maintain its size

Next button changes its size when its label is changed from/to "Continue"
to/from "Create", which could be a big problem depending on the locale
of the user. Imagine user clicking very rapidly on the button to quickly
create a box and hitting 'Back' button at the last page even though
she/he never moved the cursor.

Review of attachment 226092 [details] [review]:

I haven't tested this, but looks good

Review of attachment 226094 [details] [review]:

I'm not sure at all this one will work right in all cases, I don't know what width_chars is doing exactly for non-fixed fonts. For example, with DejaVu Sans/Serif, a single H is larger than 'ii'
Creating a dummy button with the "Done" label and adding it to the size group from the first patch should address this.

Review of attachment 226094 [details] [review]:

I'm not sure at all this one will work right in all cases, I don't know what width_chars is doing exactly for non-fixed fonts. For example, with DejaVu Sans/Serif, a single H is larger than 'ii'
Creating a dummy button with the "Done" label and adding it to the size group from the first patch should address this.

Review of attachment 226092 [details] [review]:

I have tested it, and noticed a small change that can be done:

"GtkSizeGroup objects are referenced by each widget in the size group, so once you have added all widgets to a GtkSizeGroup, you can drop the initial reference to the size group with g_object_unref()"

diff --git a/src/wizard.vala b/src/wizard.vala
index 8d1f3f6..dde93c5 100644
--- a/src/wizard.vala
+++ b/src/wizard.vala
@@ -19,7 +19,6 @@
     private Gtk.Button cancel_button;
     private Gtk.Button back_button;
     private Gtk.Button next_button;
-    private Gtk.SizeGroup toolbar_sizegroup;
     private Boxes.WizardSource wizard_source;
     private WizardSummary summary;
     private CollectionSource? source;
Stage this hunk [y,n,q,a,d,/,j,J,g,e,?]? n
@@ -601,7 +600,7 @@ private void setup_ui () {
         toolbar.toolbar_style = Gtk.ToolbarStyle.TEXT;
         hbox.pack_start (toolbar, true, true, 0);
 
-        toolbar_sizegroup = new Gtk.SizeGroup (Gtk.SizeGroupMode.HORIZONTAL);
+        var toolbar_sizegroup = new Gtk.SizeGroup (Gtk.SizeGroupMode.HORIZONTAL);
         cancel_button = toolbar.add_button (null, _("_Cancel"), false) as Gtk.Button;
         cancel_button.use_underline = true;
         cancel_button.clicked.connect (() => {

Created attachment 226108 [details] [review]
wizard: Next button should maintain its size

Next button changes its size when its label is changed from/to "Continue"
to/from "Create", which could be a big problem depending on the locale
of the user. Imagine user clicking very rapidly on the button to quickly
create a box and hitting 'Back' button at the last page even though
she/he never moved the cursor. (commit log from Zeeshan up to this point)

This commit adds a fake button to the GtkSizeGroup containing the various
toolbar button. This fake button contains the 'Create' label so that the
size group gets the correct size if the 'Create' translation makes the button
larger than the other members of the group.

Review of attachment 226108 [details] [review]:

This patch goes on top of the other patch + the changes I suggested, without them there will be a minor conflict in the declaration of the wizard class when trying to apply it.

Review of attachment 226094 [details] [review]:

We should find out if width_chars is taking care of this and i don't think we use fixed-sized fonts here anyway.

Review of attachment 226108 [details] [review]:

This seems more like a work-around so we should only go for it once we have made sure that gtk_label_set_width_chars() doesn't work reliably for all fonts.

Created attachment 226120 [details] [review]
wizard: Separate buttons for 'Create' & 'Continue'

Next button changes its size when its label is changed from/to "Continue"
to/from "Create", which could be a big problem depending on the locale
of the user. Imagine user clicking very rapidly on the button to quickly
create a box and hitting 'Back' button at the last page even though
she/he never moved the cursor.

This patche attempts to fix the problem by keeping separate buttons for
both 'Continue' and 'Create' and instead of changing the labels, it
simply shows and hides the buttons appropriately. Since both buttons are
in the same size group, the size of buttons remains the same.

*** Bug 677173 has been marked as a duplicate of this bug. ***

Review of attachment 226120 [details] [review]:

Looks good

::: src/wizard.vala
@@ -113,3 @@
             steps.get (page).modify_fg (Gtk.StateType.NORMAL, get_color ("white"));
-
-            next_button.label = page != WizardPage.REVIEW ? _("C_ontinue") : _("C_reate");

Keeping a 'next_button' member which is initially pointing to continue_button and then points to create_button would make this patch much simpler, I don't know if this is better though?

@@ +633,3 @@
+        toolbar_sizegroup.add_widget (continue_button);
+
+

extra blank line there

Created attachment 226138 [details] [review]
wizard: Separate buttons for 'Create' & 'Continue'

V2: This one keeps the next_button as a ref to 'Create' or 'Continue' and it does help making patch simpler:

From: 1 file changed, 33 insertions(+), 19 deletions(-)
To: 1 file changed, 26 insertions(+), 8 deletions(-)

Review of attachment 226138 [details] [review]:

Looks good

Attachment 226092 [details] pushed as 4ce9e3e - wizard: Same size for all toolbar buttons
Attachment 226138 [details] pushed as cad9c0d - wizard: Separate buttons for 'Create' & 'Continue'