Merge lp:~tintou/noise/fix-interface into lp:~elementary-apps/noise/trunk

[Corentin Noël]

Fixed some problems with the interface:
 * English strings in the preference dialog
 * Lastfm string being expanded…
 * Luna padding on the statusbar
 * Media Editor dialog now uses Gtk.Dialog
 * Preference Dialog and Media Editor Dialog now uses Gtk.HeaderBar
 * Smart Playlist editor dialog now uses Gtk.HeaderBar
 * Gtk.Dialog.get_action_area has been replaced with ButtonBox
 * Set suggested action style class on "Save" buttons

Fixed some problems with the interface:
 * English strings in the preference dialog
 * Lastfm string being expanded…
 * Luna padding on the statusbar
 * Media Editor dialog now uses Gtk.Dialog
 * Preference Dialog and Media Editor Dialog now uses Gtk.HeaderBar

[Victor Martinez (victored)]

This branch looks great.

I have some questions:

1. Where is the code that uses Gtk.Popover?

2. What about using the "use-header-bar" property for Gtk.Dialog? Only for GTK >= 3.12: https://developer.gnome.org/gtk3/unstable/GtkDialog.html#GtkDialog--use-header-bar

3. Gtk.Label supports text wrapping. Granite.Widgets.WrapLabel shouldn't be necessary.

4. Cast the value of Gtk.Dialog.get_content_area to Gtk.Container instead of the implementation-specific Gtk.Box.

review: Needs Information

[lp:~tintou/noise/fix-interface]

1565. By Corentin Noël on 2014-03-15

media editor: Now uses a Gtk.Dialog with HeaderBar.

1566. By Corentin Noël on 2014-03-15

preference dialog: use Headerbar for better looking and consistency

1567. By Corentin Noël on 2014-03-15

dependencies: Now required gtk+ >= 3.11.6 and vala >= 0.23.2

1568. By Corentin Noël on 2014-03-15

media editor: Using Gtk+ 3.12 integrated HeaderBar

[Corentin Noël (tintou)]

Thanks for the review!

1. Indeed, I wanted to say Gtk.Dialog instead of Gtk.Popover, it's fixed in trunk.

2. I didn't know that this property existed, I've pushed some revisions to use this property instead.

3. I tried with Gtk.Label but as the dialog has no size limit it doesn't wrap the label, Granite WrapLabel is needed.

4. Indeed, fixed.

[Danielle Foré (danrabbit)]

Hrm, I'm not sure I like removing the close button from the bottom right of dialogs. I think this creates an inconsistency since gtk.dialog retains the explicit buttons, but removes the close window decoration instead. I would agree that it's better to retain the label button since it an give a more explicit explanation of what closing the dialog does (maybe not applicable in this particular dialog, but in the scheme of platform consistency it is important in other dialogs).

I think we should:
1. revert the window decoration/close button situation in the prefs dialog
2. consider adding a "Cancel" button to the media editor and use the standard gtk dialog without a window decoration.

[Victor Martinez (victored)]

The code looks good at this point.

I believe you will implement Dan's suggestions before merging. If so please consider this as well:

1. Leave a single space after casts: (TypeName)var => (TypeName) var
2. Allocate the parent container size before packing the label or setting the text to avoid issues with text wrapping: https://developer.gnome.org/gtk3/unstable/GtkLabel.html#gtk-label-set-line-wrap
3. Gtk.Dialog.get_action_area has been deprecated since GTK 3.12: https://developer.gnome.org/gtk3/unstable/GtkDialog.html#gtk-dialog-get-action-area

[lp:~tintou/noise/fix-interface]

1569. By Corentin Noël on 2014-03-16

media editor and preferences dialog: reverted close button deletion

1570. By Corentin Noël on 2014-03-16

smartplaylisteditor: removed decorated window

1571. By Corentin Noël on 2014-03-16

Gtk 3.12 does pack_end on headerbar on an other way than previously, switched back to the original disposition.

1572. By Corentin Noël on 2014-03-16

fixed style

[Corentin Noël (tintou)]

Daniel:
 I reverted the changes, there is now a close button allowing consistency, I also added SUGGESTED_ACTION for button with a "Save" label.

Victor:
 1. Okay, changed.
 2. Hmm, it's something that could be investigated in a next branch.
 3. Yes, I replaced this with a ButtonBox in the content_area.

[lp:~tintou/noise/fix-interface]

1573. By Corentin Noël on 2014-03-16

Sync with trunk, resolved conflicts

[Danielle Foré (danrabbit)]

I think I'd still prefer using the regular gtk.dialog csd instead of adding a headerbar, but I'll approve this merge :)

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-03-07 15:52:49 +0000
+++ CMakeLists.txt	2014-03-16 21:07:17 +0000
@@ -1,7 +1,7 @@
 # check http://elementaryos.org/docs/developer-guide/cmake/simple-project
 
-cmake_minimum_required (VERSION 2.6)
-cmake_policy (VERSION 2.6)
+cmake_minimum_required (VERSION 2.8)
+cmake_policy (VERSION 2.8)
 project (noise C)
 list (APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
 
@@ -74,7 +74,7 @@
   gee-0.8
   libpeas-1.0
   libpeas-gtk-1.0
-  gtk+-3.0>=3.10
+  gtk+-3.0>=3.11.6
   granite
   gstreamer-1.0
   gstreamer-tag-1.0
@@ -150,7 +150,7 @@
 
 find_package (Vala REQUIRED)
 include (ValaVersion)
-ensure_vala_version ("0.22.0" MINIMUM)
+ensure_vala_version ("0.23.2" MINIMUM)
 include (ValaPrecompile)
 
 set (DEPS_LIBRARIES ${DEPS_LIBRARIES} -lm)
@@ -165,4 +165,4 @@
 add_subdirectory (plugins)
 add_subdirectory (po)
 add_subdirectory (data)
-add_subdirectory (images)
 \ No newline at end of file
+add_subdirectory (images)
 
=== modified file 'plugins/LastFM/PreferencesSection.vala'
--- plugins/LastFM/PreferencesSection.vala	2013-12-01 14:35:04 +0000
+++ plugins/LastFM/PreferencesSection.vala	2014-03-16 21:07:17 +0000
@@ -55,10 +55,10 @@
             login_button.set_tooltip_text(_("Click to redo the Last.fm login process"));
         }
  
-        var label = new Gtk.Label (_("To allow for Last.fm integration, you must give permission to %s. You only need to do this once.").printf(program_name));
+        var label = new Granite.Widgets.WrapLabel (_("To allow for Last.fm integration, you must give permission to %s. You only need to do this once.").printf(program_name));
         label.xalign = 0.0f;
         label.halign = Gtk.Align.START;
-        label.set_line_wrap (true);
+        label.expand = false;
         
         page.add_full_option (label, ref row);
         page.add_full_option (login_button, ref row);
@@ -107,4 +107,4 @@
             }
         }
     }
-}
+}
 \ No newline at end of file
 
=== modified file 'src/Dialogs/MediaEditor.vala'
--- src/Dialogs/MediaEditor.vala	2014-03-07 13:43:48 +0000
+++ src/Dialogs/MediaEditor.vala	2014-03-16 21:07:17 +0000
@@ -35,11 +35,7 @@
  *       CDs before importing their media to the library).
  */
 
-#if USE_GRANITE_DECORATED_WINDOW
-public class Noise.MediaEditor : Granite.Widgets.LightWindow {
-#else
-public class Noise.MediaEditor : Gtk.Window {
-#endif
+public class Noise.MediaEditor : Gtk.Dialog {
     LyricFetcher lf;
     
     Gee.LinkedList<int> _allMedias;
@@ -51,7 +47,8 @@
     private Gee.HashMap<string, FieldEditor> fields;// a hashmap with each property and corresponding editor
     private Gtk.TextView lyricsText;
     
-    private Gtk.Button _save;
+    private Gtk.Button save_button;
+    private Gtk.Button close_button;
     
     private Gtk.Label lyricsInfobarLabel;
     private Library library;
@@ -59,6 +56,7 @@
     public signal void medias_saved (Gee.LinkedList<int> medias);
     
     public MediaEditor (Gee.LinkedList<int> allMedias, Gee.LinkedList<int> medias, Library library) {
+        Object (use_header_bar: 1);
         this.library = library;
         this.window_position = Gtk.WindowPosition.CENTER;
         this.type_hint = Gdk.WindowTypeHint.DIALOG;
@@ -75,9 +73,12 @@
         _medias = medias;
         
         stack = new Gtk.Stack ();
+        
         var stack_switcher = new Gtk.StackSwitcher ();
         stack_switcher.set_stack (stack);
         stack_switcher.halign = Gtk.Align.CENTER;
+        
+        ((Gtk.HeaderBar) get_header_bar ()).set_custom_title (stack_switcher);
 
         stack.add_titled (createBasicContent (), "metadata", _("Metadata"));
         if(_medias.size == 1)
@@ -87,29 +88,27 @@
 
         var arrows = new Granite.Widgets.NavigationArrows ();
 
-        _save = new Gtk.Button.with_label (_(STRING_SAVE));
-        _save.set_size_request (85, -1);
+        save_button = new Gtk.Button.with_label (_(STRING_SAVE));
+        save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_SUGGESTED_ACTION);
 
-        _save.valign = arrows.valign = Gtk.Align.END;
+        close_button  = new Gtk.Button.with_label (_("Close"));
 
         var buttons = new Gtk.ButtonBox (Gtk.Orientation.HORIZONTAL);
-        buttons.margin_top = 12;
         buttons.set_layout (Gtk.ButtonBoxStyle.END);
+        buttons.set_spacing (6);
 
         buttons.pack_start (arrows, false, false, 0);
-        buttons.pack_end (_save, false, false, 0);
-
+        buttons.pack_end (close_button, false, false, 0);
+        buttons.pack_end (save_button, false, false, 0);
         buttons.set_child_secondary (arrows, true);
 
-        var content = new Gtk.Grid ();
-        content.orientation = Gtk.Orientation.VERTICAL;
-        content.margin = 12;
-
-        content.add (stack_switcher);
-        content.add (stack);
-        content.add (buttons);
-
-        this.add (content);
+        var main_grid = new Gtk.Grid ();
+        main_grid.attach (stack, 0, 0, 1, 1);
+        main_grid.attach (buttons, 0, 1, 1, 1);
+
+        var content = get_content_area () as Gtk.Container;
+        content.margin_left = content.margin_right = 12;
+        content.add (main_grid);
 
         this.show_all();
 
@@ -122,9 +121,10 @@
             fetch_lyrics.begin (false);
         }
 
-        arrows.previous_clicked.connect(previousClicked);
-        arrows.next_clicked.connect(nextClicked);
-        _save.clicked.connect(saveClicked);
+        arrows.previous_clicked.connect (previousClicked);
+        arrows.next_clicked.connect (nextClicked);
+        save_button.clicked.connect (saveClicked);
+        close_button.clicked.connect (() => {destroy ();});
     }
     
     public Gtk.Box createBasicContent () {
@@ -719,25 +719,4 @@
         
         info.set_markup(text);
     }
-}
-
-/*public class Noise.DoubleSpinButton : HBox {
-    private SpinButton spin1;
-    private SpinButton spin2;
-    
-    public DoubleSpinButton(double val1, double val2, double maxVal) {
-        spin1 = new SpinButton.with_range(0.0, maxVal, 1.0);
-        spin2 = new SpinButton.with_range(0.0, maxVal, 1.0);
-        
-        spin1.set_value(val1);
-        spin2.set_value(val2);
-    }
-    
-    public double getVal1() {
-        return spin1.get_value();
-    }
-    
-    public double getVal2() {
-        return spin2.get_value();
-    }
-}*/
 \ No newline at end of file
+}
 \ No newline at end of file
 
=== modified file 'src/Dialogs/PreferencesWindow.vala'
--- src/Dialogs/PreferencesWindow.vala	2014-03-07 13:43:48 +0000
+++ src/Dialogs/PreferencesWindow.vala	2014-03-16 21:07:17 +0000
@@ -39,6 +39,7 @@
     private int index = 0;
 
     public PreferencesWindow (LibraryWindow lw) {
+        Object (use_header_bar: 1);
         build_ui (lw);
 
         lw.add_preference_page.connect ((page) => {add_page (page);});
@@ -71,34 +72,39 @@
         return index;
     }
 
-
     public void remove_section (int index) {
         var section = sections.get (index);
         section.destroy ();
         sections.unset (index);
     }
 
-
     private void build_ui (Gtk.Window parent_window) {
-        set_size_request (MIN_WIDTH, MIN_HEIGHT);
-
         // Window properties
         title = _("Preferences");
+        set_size_request (MIN_WIDTH, MIN_HEIGHT);
         resizable = false;
         window_position = Gtk.WindowPosition.CENTER;
         type_hint = Gdk.WindowTypeHint.DIALOG;
         transient_for = parent_window;
-        var main_grid = new Gtk.Grid ();
+
         main_stack = new Gtk.Stack ();
         main_stackswitcher = new Gtk.StackSwitcher ();
         main_stackswitcher.set_stack (main_stack);
-        main_stackswitcher.halign = Gtk.Align.CENTER;
-        main_grid.attach (main_stackswitcher, 0, 0, 1, 1);
-        main_grid.attach (main_stack, 0, 1, 1, 1);
-        main_grid.hexpand = true;
-
-        ((Gtk.Box)get_content_area()).add (main_grid);
-        add_button (_("Close"), Gtk.ResponseType.ACCEPT);
+
+        var close_button = new Gtk.Button.with_label (_("Close"));
+        close_button.clicked.connect (() => {this.destroy ();});
+
+        var button_box = new Gtk.ButtonBox (Gtk.Orientation.HORIZONTAL);
+        button_box.set_layout (Gtk.ButtonBoxStyle.END);
+        button_box.pack_end (close_button);
+        button_box.margin_right = 12;
+
+        var main_grid = new Gtk.Grid ();
+        main_grid.attach (main_stack, 0, 0, 1, 1);
+        main_grid.attach (button_box, 0, 1, 1, 1);
+
+        ((Gtk.HeaderBar) get_header_bar ()).set_custom_title (main_stackswitcher);
+        ((Gtk.Container) get_content_area ()).add (main_grid);
     }
 }
 
@@ -157,18 +163,12 @@
 #if HAVE_LIBNOTIFY
         show_notifications_switch = new Gtk.Switch ();
         main_settings.schema.bind("show-notifications", show_notifications_switch, "active", SettingsBindFlags.DEFAULT);
-        page.add_option (new Gtk.Label (_("Show notifications:")), show_notifications_switch, ref row);
+        page.add_option (new Gtk.Label (_("Notifications:")), show_notifications_switch, ref row);
 #endif
 
-        string hide_on_close_desc;
-        if (LibraryWindow.minimize_on_close ())
-            hide_on_close_desc = _("Minimize window when a song is being played:");
-        else
-            hide_on_close_desc = _("Hide window when a song is being played:");
-
         hide_on_close_switch = new Gtk.Switch ();
         main_settings.schema.bind("close-while-playing", hide_on_close_switch, "active", SettingsBindFlags.INVERT_BOOLEAN);
-        page.add_option (new Gtk.Label (hide_on_close_desc), hide_on_close_switch, ref row);
+        page.add_option (new Gtk.Label (_("Continue playback when closed:")), hide_on_close_switch, ref row);
         
     }
 }
 \ No newline at end of file
 
=== modified file 'src/Dialogs/SmartPlaylistEditor.vala'
--- src/Dialogs/SmartPlaylistEditor.vala	2014-02-02 00:10:45 +0000
+++ src/Dialogs/SmartPlaylistEditor.vala	2014-03-16 21:07:17 +0000
@@ -17,11 +17,7 @@
  *              Corentin Noël <[e-mail]>
  */
 
-#if USE_GRANITE_DECORATED_WINDOW
-public class Noise.SmartPlaylistEditor : Granite.Widgets.LightWindow {
-#else
-public class Noise.SmartPlaylistEditor : Window {
-#endif
+public class Noise.SmartPlaylistEditor : Gtk.Dialog {
 
     SmartPlaylist sp;
     private bool is_new = false;
@@ -32,13 +28,13 @@
     private Gtk.Grid queries_grid;
     private Gtk.CheckButton limit_check;
     private Gtk.SpinButton limit_spin;
-    private Gtk.Button add_button;
+    private Gtk.Button adding_button;
     private Gee.ArrayList<SmartPlaylistEditorQuery> queries_list;
     private int row = 0;
     private Library library;
 
-    public SmartPlaylistEditor(SmartPlaylist? sp = null, Library library) {
-        
+    public SmartPlaylistEditor (SmartPlaylist? sp = null, Library library) {
+        Object (use_header_bar: 1);
         this.title = _("Smart Playlist Editor");
         this.library = library;
         
@@ -68,6 +64,7 @@
             name_entry.text = sp.name;
         
         var match_grid = new Gtk.Grid ();
+        match_grid.column_spacing = 12;
         var match_label = new Gtk.Label (_("Match"));
         match_combobox = new Gtk.ComboBoxText ();
         match_combobox.insert_text (0, _("any"));
@@ -85,12 +82,14 @@
         queries_list = new Gee.ArrayList<SmartPlaylistEditorQuery> ();
         queries_grid = new Gtk.Grid ();
         queries_grid.column_spacing = 12;
+        queries_grid.row_spacing = 6;
         queries_grid.expand = true;
         
-        add_button = new Gtk.Button.with_label (_("Add"));
+        adding_button = new Gtk.Button.with_label (_("Add"));
         
         /* create extra option: limiter */
         var limiter_grid = new Gtk.Grid ();
+        limiter_grid.column_spacing = 12;
         limit_check = new Gtk.CheckButton.with_label (_("Limit to"));
         limit_spin = new Gtk.SpinButton.with_range (0, 500, 10);
         var limit_label = new Gtk.Label (_("items"));
@@ -114,6 +113,7 @@
         var button_box = new Gtk.ButtonBox (Gtk.Orientation.HORIZONTAL);
         button_box.spacing = 6;
         save_button = new Gtk.Button.with_label (_(STRING_SAVE));
+        save_button.get_style_context ().add_class (Gtk.STYLE_CLASS_SUGGESTED_ACTION);
         var close_button = new Gtk.Button.with_label (_(STRING_CANCEL));
         button_box.set_layout (Gtk.ButtonBoxStyle.END);
         button_box.pack_end (close_button, false, false, 0);
@@ -121,7 +121,7 @@
         
         main_grid = new Gtk.Grid ();
         main_grid.expand = true;
-        main_grid.margin = 12;
+        main_grid.margin_left = main_grid.margin_right = 12;
         main_grid.column_spacing = 12;
         main_grid.row_spacing = 6;
         main_grid.attach (name_label, 0, 0, 3, 1);
@@ -132,7 +132,7 @@
         main_grid.attach (options_label, 0, 5, 3, 1);
         main_grid.attach (limiter_grid, 0, 6, 3, 1);
         main_grid.attach (button_box, 0, 7, 3, 1);
-        add (main_grid);
+        ((Gtk.Container) get_content_area ()).add (main_grid);
         
         save_button.clicked.connect (save_click);
         close_button.clicked.connect (close_click);
@@ -152,15 +152,15 @@
             row++;
         }
         
-        queries_grid.attach (add_button, 0, row, 1, 1);
+        queries_grid.attach (adding_button, 0, row, 1, 1);
         if(sp.queries ().size == 0) {
             add_row ();
         }
         foreach(SmartPlaylistEditorQuery speq in queries_list) {
             speq.field_changed (false);
         }
-        add_button.clicked.connect (add_button_click);
-        add_button.show ();
+        adding_button.clicked.connect (add_button_click);
+        adding_button.show ();
         // Validate initial state
         name_changed ();
     }
@@ -183,15 +183,15 @@
     }
 
     public void add_row () {
-        if (add_button.parent != null)
-            queries_grid.remove (add_button);
+        if (adding_button.parent != null)
+            queries_grid.remove (adding_button);
         var editor_query = new SmartPlaylistEditorQuery (new SmartQuery());
         editor_query.removed.connect (() => {queries_list.remove (editor_query);});
         editor_query.changed.connect (() => {if (!queries_list.contains (editor_query)) queries_list.add (editor_query);});
         queries_grid.attach (editor_query.grid, 0, row, 1, 1);
         editor_query.grid.show ();
         row++;
-        queries_grid.attach (add_button, 0, row, 1, 1);
+        queries_grid.attach (adding_button, 0, row, 1, 1);
         editor_query.field_changed (false);
     }
     
@@ -246,8 +246,7 @@
         _q = q;
         
         comparators = new GLib.HashTable<int, SmartQuery.ComparatorType> (null, null);
-        
-        grid = new Gtk.Grid ();
+
         field_combobox = new Gtk.ComboBoxText ();
         comparator_combobox = new Gtk.ComboBoxText();
         value_entry = new Gtk.Entry ();
@@ -286,9 +285,10 @@
         } else {
             _valueNumerical.set_value (int.parse (q.value));
         }
-            
         _units = new Gtk.Label ("");
-        
+
+        grid = new Gtk.Grid ();
+        grid.column_spacing = 12;
         grid.hexpand = true;
         grid.attach (field_combobox, 0, 0, 1, 1);
         grid.attach (comparator_combobox, 1, 0, 1, 1);
@@ -441,4 +441,4 @@
         return (compared == SmartQuery.FieldType.LAST_PLAYED || compared == SmartQuery.FieldType.DATE_ADDED
                 || compared == SmartQuery.FieldType.DATE_RELEASED);
     }
-}
+}
 \ No newline at end of file
 
=== modified file 'src/LibraryWindow.vala'
--- src/LibraryWindow.vala	2014-03-14 15:48:01 +0000
+++ src/LibraryWindow.vala	2014-03-16 21:07:17 +0000
@@ -285,8 +285,8 @@
         headerbar.pack_start (nextButton);
         headerbar.pack_start (viewSelector);
         headerbar.set_custom_title (topDisplayBin);
+        headerbar.pack_end (((Noise.App) GLib.Application.get_default ()).create_appmenu (settingsMenu));
         headerbar.pack_end (searchField);
-        headerbar.pack_end (((Noise.App) GLib.Application.get_default ()).create_appmenu (settingsMenu));
         headerbar.show_all ();
 
 
@@ -1130,7 +1130,7 @@
             preferences = new PreferencesWindow(this);
         preferences.show_all ();
         preferences.run ();
-        preferences.hide ();
+        preferences = null;
     }
 
     public void setMusicFolder(string folder) {
 
=== modified file 'src/Widgets/StatusBar.vala'
--- src/Widgets/StatusBar.vala	2014-02-02 11:47:19 +0000
+++ src/Widgets/StatusBar.vala	2014-03-16 21:07:17 +0000
@@ -218,12 +218,6 @@
     private class InfoPanelChooser : SimpleOptionChooser {
 
         public InfoPanelChooser () {
-            //To avoid the InfoPanelChooser to overlap with the window resize grip            
-            //Get the grip width
-            int resize_grip_width;
-            App.main_window.style_get ("resize-grip-width", out resize_grip_width);
-            //Use the actual grip width as right margin
-            margin_right = resize_grip_width;
             var info_panel_show = Icons.PANE_SHOW_SYMBOLIC.render_image (Gtk.IconSize.MENU);
             var info_panel_hide = Icons.PANE_HIDE_SYMBOLIC.render_image (Gtk.IconSize.MENU);