GNOME Bugzilla – Bug 731580
sharing: Update from mockups
Last modified: 2014-06-23 10:10:06 UTC
See https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/sharing/media-sharing.png
Created attachment 278347 [details] [review] sharing: Change explanatory text to match mockups
Created attachment 278348 [details] [review] sharing: Move Media Sharing master toggle to header bar As per mockups
Created attachment 278349 [details] [review] sharing: Rename header label in Media Sharing As per mockups
Created attachment 278350 [details] [review] sharing: Use GtkListBox instead of treeview for media folders As per mockups
Review of attachment 278350 [details] [review]: In general I think we need more padding in the rows to match the main panel rows size. Also, when we don't have any shared folders I think we should have a label in the box instead of the the empty list with a '+' row. ::: panels/sharing/cc-sharing-panel.c @@ +453,3 @@ gtk_widget_hide (dialog); + box = (GtkListBox *) gtk_builder_get_object (priv->builder, Could use WID() @@ +514,1 @@ + box = (GtkWidget *) gtk_builder_get_object (priv->builder, WID() @@ +565,3 @@ + + default: + return g_themed_icon_new_with_default_fallbacks ("folder-symbolic"); ICON_NAME_FOLDER ? @@ +655,3 @@ + +static void +cc_sharing_panel_list_box_update_header (GtkListBoxRow *row, We already have cc_sharing_panel_main_list_box_udpate_header() with the same code. We could rename it to something generic and re-use here. @@ +706,3 @@ gtk_switch_set_active (GTK_SWITCH (WID ("share-media-switch")), enabled); + box = (GtkWidget *) gtk_builder_get_object (priv->builder, WID() ? ::: panels/sharing/sharing.ui @@ +972,3 @@ <property name="vexpand">True</property> <property name="shadow_type">in</property> + <property name="vscrollbar-policy">never</property> I'm afraid we'll need a scrollbar here. We should perhaps add it dynamically according to the number of rows we have like we do in the region panel for the input sources list.
(In reply to comment #5) > Review of attachment 278350 [details] [review]: > > In general I think we need more padding in the rows to match the main panel > rows size. I'm using a 12 pixels spacing, and all the widgets are also separated by 12 pixels. I'll check that again once I can run the panel again (changes in vino, rygel and gnome-user-share prevent me from doing that for now). > Also, when we don't have any shared folders I think we should have a label in > the box instead of the the empty list with a '+' row. Allan? > ::: panels/sharing/cc-sharing-panel.c > @@ +453,3 @@ > gtk_widget_hide (dialog); > > + box = (GtkListBox *) gtk_builder_get_object (priv->builder, > > Could use WID() Done. > @@ +514,1 @@ > + box = (GtkWidget *) gtk_builder_get_object (priv->builder, > > WID() Done. > @@ +565,3 @@ > + > + default: > + return g_themed_icon_new_with_default_fallbacks ("folder-symbolic"); > > ICON_NAME_FOLDER ? Was a bug in GTK+: https://bugzilla.gnome.org/show_bug.cgi?id=731908 > @@ +655,3 @@ > + > +static void > +cc_sharing_panel_list_box_update_header (GtkListBoxRow *row, > > We already have cc_sharing_panel_main_list_box_udpate_header() with the same > code. We could rename it to something generic and re-use here. Removed the duplicate. > @@ +706,3 @@ > gtk_switch_set_active (GTK_SWITCH (WID ("share-media-switch")), enabled); > > + box = (GtkWidget *) gtk_builder_get_object (priv->builder, > > WID() ? Done. > ::: panels/sharing/sharing.ui > @@ +972,3 @@ > <property name="vexpand">True</property> > <property name="shadow_type">in</property> > + <property name="vscrollbar-policy">never</property> > > I'm afraid we'll need a scrollbar here. We should perhaps add it dynamically > according to the number of rows we have like we do in the region panel for the > input sources list. Done.
Created attachment 278763 [details] [review] sharing: Use GtkListBox instead of treeview for media folders As per mockups
Created attachment 278764 [details] [review] sharing: Use GtkListBox instead of treeview for media folders As per mockups
Attachment 278347 [details] pushed as c5f991f - sharing: Change explanatory text to match mockups Attachment 278348 [details] pushed as f81398c - sharing: Move Media Sharing master toggle to header bar Attachment 278349 [details] pushed as ca84cee - sharing: Rename header label in Media Sharing Attachment 278764 [details] pushed as 9b4fcd6 - sharing: Use GtkListBox instead of treeview for media folders