GNOME Bugzilla – Bug 663631
Improve the preferences dialog
Last modified: 2012-02-06 19:11:22 UTC
Created attachment 200996 [details] [review] improve the preferences dialog Attaching a work-in-progress patch which improves the preferences dialog by adding tooltips to the widgets inside the dialog, and updates the sensitivity of the remove and clear buttons based on the content of the URI model. There are still a few issues to solve: * clear and close buttons use stock items and have the same mnemonic, at least in English * the code to save the dialog settings does a string comparison which explicitly matches the string ‘Any’, which is marked as translatable, so it should be using the translated string in the code too * the interface combo box has no active entry on startup, despite setting it in the gtkbuilder file, and forcing the setting through code also does not work Solving the translatable string problem should be straightforward. One way of solving the duplicate mnemonics would be to remove the clear button from the dialog, allow multiple selection and delete the selected rows from the model. The inactive combo box entry might be caused by the context-available and context-unavailable signal handlers not checking for an active item and updating the combo box appropriately.
Created attachment 201010 [details] [review] fix translation of Any string Fixed the translatable string.
Review of attachment 201010 [details] [review]: I wonder if we shouldn't move to the new +/- style thing, given that gnome-sharing isn't going to appear soon. Code is missing curlies for single-line statements, also the patch does do some unrelated things :) ::: src/ui/rygel-media-pref-section.vala @@ +56,3 @@ + this.tree_selection = (TreeSelection) + builder.get_object (TREE_SELECTION); + assert (this.tree_selection != null); Indentation is wrong and the "as" looks way cooler ;) ::: src/ui/rygel-network-pref-section.vala @@ +58,3 @@ var iface = this.iface_entry.get_active_text (); + // FIXME: This check does not work if ANY_NETWORK has been translated! The "Any" entry should always be the first. Can't we check that instead of doing the string compare?
Created attachment 206003 [details] [review] updated patch based on comment #2 I added the +/- Toolbar and ToolButtons, switched the Boxes to Grids, changed the interface handling to treat the first item specially, removed the clear button, used the ‘as’ operator, added the curly braces and hopefully fixed the indentation.
Review of attachment 206003 [details] [review]: Small issues: +/- buttons are too large and the - button has the + image. I can fix that, but for some reason there's a line below the toolbar that I can't really explain.
(In reply to comment #4) > Review of attachment 206003 [details] [review]: > > Small issues: +/- buttons are too large and the - button has the + image. I can > fix that, but for some reason there's a line below the toolbar that I can't > really explain. → Wrong toolbar style. Will also fix.