Code review comment for lp:~cr3/checkbox/411526

Revision history for this message
Javier Collado (javier.collado) wrote :

Hello,

The change looks fine, the only thing that I'd change is the expand property for the upper part of the dialog to false since it takes half the available space otherwise (I don't know why, but it still takes more space than strictly needed).

Regarding the horizontal scrollbar. I tried to use a really long test suite description and it wasn't wrapped to the next line, but it was just partially displayed. In this case I believe it's much better to have a scrollbar that it's displayed only when needed than not having it at all, right?

=== modified file 'gtk/checkbox-gtk.glade'
--- gtk/checkbox-gtk.glade 2009-11-05 16:09:25 +0000
+++ gtk/checkbox-gtk.glade 2009-11-06 08:19:54 +0000
@@ -211,7 +211,7 @@
   </widget>
   <packing>
     <property name="padding">0</property>
- <property name="expand">True</property>
+ <property name="expand">False</property>
     <property name="fill">True</property>
   </packing>
        </child>
@@ -220,7 +220,7 @@
   <widget class="GtkScrolledWindow" id="scrolledwindow_options_list">
     <property name="visible">True</property>
     <property name="can_focus">True</property>
- <property name="hscrollbar_policy">GTK_POLICY_NEVER</property>
+ <property name="hscrollbar_policy">GTK_POLICY_AUTOMATIC</property>
     <property name="vscrollbar_policy">GTK_POLICY_AUTOMATIC</property>
     <property name="shadow_type">GTK_SHADOW_NONE</property>
     <property name="window_placement">GTK_CORNER_TOP_LEFT</property>
Best regards,
    Javier

P.S. I'm still wondering which editor and/or config do you use. I wasn't able to save any change with glade (neither v2 nor v3) without completely changing the whole file.

review: Needs Fixing

« Back to merge proposal