Hi,
I fixed up the layout and alignment of widgets in the class builder plugin. I also fixed a small bug which was causing an assertion warning.
There's 3 patches:
#0001 The UI cleanup using a GtkTable, for side-by-side before and after screenshots, see [1][2][3]
#0002 I forgot to update comments on the helper functions I added and there was a place where I wasn't using one of the helper functions where I should have.
#0003 Fix a bug with the order of function calls which was causing an assertion warning (from g_return_if_fail) when there are no documents currently opened.
I used git format-patch to create the patches from my commits, I hope the format is OK.
[1] http://codebrainz.ca/images/classbuilder/c++_before_after.png [2] http://codebrainz.ca/images/classbuilder/gtk+_before_after.png [3] http://codebrainz.ca/images/classbuilder/php_before_after.png
Cheers, Matthew Brush
On Wed, 16 Mar 2011 10:43:08 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
I fixed up the layout and alignment of widgets in the class builder plugin. I also fixed a small bug which was causing an assertion warning.
There's 3 patches:
#0001 The UI cleanup using a GtkTable, for side-by-side before and after screenshots, see [1][2][3]
...
[1] http://codebrainz.ca/images/classbuilder/c++_before_after.png [2] http://codebrainz.ca/images/classbuilder/gtk+_before_after.png [3] http://codebrainz.ca/images/classbuilder/php_before_after.png
My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good. Increasing horizontal separation between radio buttons - not sure, but perhaps more usable. Unaligning entry fields seems bad. Removing filetype from dialog title (even though each dialog has different fields) seems bad. Increasing vertical separations seems bad (out of keeping with Geany etc).
I didn't look at the code BTW, only the screenshots. Also, if there are Gnome HIG reasons to support you, fair enough, but consistency with Geany is probably most important.
Regards, Nick
Le 16/03/2011 18:51, Nick Treleaven a écrit :
On Wed, 16 Mar 2011 10:43:08 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
I fixed up the layout and alignment of widgets in the class builder plugin. I also fixed a small bug which was causing an assertion warning.
There's 3 patches:
#0001 The UI cleanup using a GtkTable, for side-by-side before and after screenshots, see [1][2][3]
...
[1] http://codebrainz.ca/images/classbuilder/c++_before_after.png [2] http://codebrainz.ca/images/classbuilder/gtk+_before_after.png [3] http://codebrainz.ca/images/classbuilder/php_before_after.png
My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Not sure, isn't "Foo Bar" the classic style in Geany?
Increasing horizontal separation between radio buttons - not sure, but perhaps more usable. Unaligning entry fields seems bad. Removing filetype from dialog title (even though each dialog has different fields) seems bad. Increasing vertical separations seems bad (out of keeping with Geany etc).
Actually the screenshots seems inverted: the new is on the left, old on the right (as far as I can tell from the current in my Geany version -- though I thought the fields were aligned...).
Cheers, Colomban
On Wed, 16 Mar 2011 18:58:17 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Not sure, isn't "Foo Bar" the classic style in Geany?
No, look at the Preferences dialog. The HIG supports this too.
Increasing horizontal separation between radio buttons - not sure, but perhaps more usable. Unaligning entry fields seems bad. Removing filetype from dialog title (even though each dialog has different fields) seems bad. Increasing vertical separations seems bad (out of keeping with Geany etc).
Actually the screenshots seems inverted: the new is on the left, old on the right (as far as I can tell from the current in my Geany version -- though I thought the fields were aligned...).
Aha... I just read before/after so assumed left would be before. In that case, patch looks good, except wrong capitalization of labels. Sorry for the confusion.
Regards, Nick
Le 16/03/2011 19:06, Nick Treleaven a écrit :
On Wed, 16 Mar 2011 18:58:17 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Not sure, isn't "Foo Bar" the classic style in Geany?
No, look at the Preferences dialog. The HIG supports this too.
Right, my bad, I looked at menus items. Though, the Toolbar preference tab is wrong then.
On Wed, 16 Mar 2011 19:10:31 +0100 Colomban Wendling lists.ban@herbesfolles.org wrote:
Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Not sure, isn't "Foo Bar" the classic style in Geany?
No, look at the Preferences dialog. The HIG supports this too.
Right, my bad, I looked at menus items.
Menu items are capitalized differently (HIG again).
Though, the Toolbar preference tab is wrong then.
True.
Regards, Nick
On Wed, 16 Mar 2011 18:13:09 +0000 Nick Treleaven nick.treleaven@btinternet.com wrote:
Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Not sure, isn't "Foo Bar" the classic style in Geany?
No, look at the Preferences dialog. The HIG supports this too.
...
Though, the Toolbar preference tab is wrong then.
True.
Now fixed.
Regards, Nick
On 03/16/11 10:51, Nick Treleaven wrote:
On Wed, 16 Mar 2011 10:43:08 -0700 My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Fixed in the attached patch (see below).
Increasing horizontal separation between radio buttons - not sure, but perhaps more usable. Unaligning entry fields seems bad. Removing filetype from dialog title (even though each dialog has different fields) seems bad. Increasing vertical separations seems bad (out of keeping with Geany etc).
Sorry for the unfortunate filenames I chose, I should have said the left side is New the right side is Old (reverse of what the filenames imply).
I didn't look at the code BTW, only the screenshots. Also, if there are Gnome HIG reasons to support you, fair enough, but consistency with Geany is probably most important.
Cheers, Matthew Brush
On Wed, 16 Mar 2011 11:07:52 -0700 Matthew Brush mbrush@codebrainz.ca wrote:
On 03/16/11 10:51, Nick Treleaven wrote:
On Wed, 16 Mar 2011 10:43:08 -0700 My comments: Changing 'Foo Bar:' to 'Foo bar:' in labels is good.
Fixed in the attached patch (see below).
Thanks.
I've applied the patches, but I didn't want to change all the translation labels to remove a ':' because this would mean unnecessary extra work for translators, so I restored the colons.
Regards, Nick
On 03/17/11 06:05, Nick Treleaven wrote:
I've applied the patches, but I didn't want to change all the translation labels to remove a ':' because this would mean unnecessary extra work for translators, so I restored the colons.
Thanks for letting me know about this, I will keep it in mind for next time.
Cheers, Matthew Brush