GNOME Bugzilla – Bug 687772
Implement the Sharing panel designs
Last modified: 2013-01-16 10:02:24 UTC
Primary goals include giving the ability to visualize all forms for shared content and connections, and to let someone control how they appear over the network. These ability have been a long-standing omission for System Settings. Mockups can be found on the wiki: https://live.gnome.org/Design/SystemSettings/Sharing
*** Bug 636206 has been marked as a duplicate of this bug. ***
*** Bug 658498 has been marked as a duplicate of this bug. ***
This mockups misses a way for entering places/folders that are omitted from recently used AND the search. Could be named "Folders/places to be excluded from any sort of indexing or history". Maybe it should require reentering the user password to view and modify for obvious reasons (although that would only increase obscurity, not real security, I know).
(In reply to comment #3) > This mockups misses a way for entering places/folders that are omitted from > recently used AND the search. That's not the job of the sharing panel but of the Privacy panel.
Sorry my comment must have ended up on the wrong bug report.
Created attachment 233165 [details] [review] Add CcHostnameEntry widget and use it in the info panel
Created attachment 233166 [details] [review] Add initial implementation of the new Sharing panel
Review of attachment 233165 [details] [review]: ::: panels/info/cc-info-panel.c @@ +437,2 @@ static gboolean +get_current_is_fallback (CcInfoPanel *self) What's that? Remove it. ::: shell/cc-hostname-entry.c @@ +205,3 @@ + +static void +cc_hostname_entry_set_property (GObject *object, Remove the unused get/set property calls. @@ +237,3 @@ +cc_hostname_entry_finalize (GObject *object) +{ + G_OBJECT_CLASS (cc_hostname_entry_parent_class)->finalize (object); Remove the chained up finalize function.
Review of attachment 233166 [details] [review]: You're missing an update to the man pages (in man/) and POTFILES.{in,skip} ::: panels/sharing/Makefile.am @@ +4,3 @@ +uidir = $(pkgdatadir)/ui/sharing +dist_ui_DATA = \ + sharing.ui Need to use GResources like the other panels. Don't forget to remove the unused defines. ::: panels/sharing/cc-remote-login.c @@ +34,3 @@ + gchar *object_path; + + connection = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, NULL); This should be async by default. Disable the widget until the first answer is retrieved, and monitor the status. @@ +101,3 @@ +cc_remote_login_set_enabled (gboolean enabled) +{ + /* not implemented yet */ Note that you should use a spinner to replace the switch when toggling it. The Bluetooth panel has an implementation of that which could be shared. ::: panels/sharing/cc-sharing-panel.c @@ +63,3 @@ + { + gtk_widget_destroy (priv->bluetooth_sharing_dialog); + priv->bluetooth_sharing_dialog = NULL; g_clear_object() @@ +96,3 @@ +cc_sharing_panel_finalize (GObject *object) +{ + G_OBJECT_CLASS (cc_sharing_panel_parent_class)->finalize (object); Remove empty chain-up function. @@ +138,3 @@ + + if (g_str_equal (widget_name, "bluetooth-sharing-button")) + cc_sharing_panel_run_dialog (self, "bluetooth-sharing-dialog"); Deduce the dialog name from the button name instead? items = g_strsplit (widget_name, "-", -1); dialog_name = g_strdup_printf ("%s-%s-dialog", items[0], items[1]); @@ +259,3 @@ + cc_sharing_panel_bind_switch_to_widgets (WID ("share-public-folder-switch"), + WID ("only-share-trusted-devices-switch"), + WID ("label3"), Rename label. @@ +464,3 @@ + +static gboolean +cc_sharing_panel_check_schema_available (CcSharingPanel *self, Pass the schemas_list to the function to avoid having to gather it multiple times. @@ +496,3 @@ + cc_sharing_panel_bind_switch_to_widgets (WID ("remote-view-switch"), + WID ("remote-control-switch"), + WID ("label21"), Rename label. @@ +503,3 @@ + WID ("approve-all-connections-switch"), + WID ("remote-control-require-password-switch"), + WID ("label22"), Ditto. ::: panels/sharing/file-share-properties.c @@ +34,3 @@ + +void +file_share_write_out_password (const char *password) Please file a bug against gnome-user-share about removing the preferences. ::: panels/sharing/gnome-sharing-panel.desktop.in.in @@ +11,3 @@ +X-GNOME-Settings-Panel=sharing +# Translators: those are keywords for the sharing control-center panel +_Keywords= Share, sharing, ssh, host, name, remote, desktop, bluetooth, obex, ::: panels/sharing/vino-preferences.c @@ +104,3 @@ + string = g_value_get_string (value); + +#if 0 Implement?
Created attachment 233252 [details] [review] Updated patch for the initial implementation of the sharing panel This patch adds support for writing Rygel configuration, although it does not yet ensure that Rygel is running when the media sharing options are enabled. This patch also fixes a problem with the screen sharing configuration in the previous patch.
Created attachment 233430 [details] [review] Add CcHostnameEntry widget and use it in the info panel (In reply to comment #8) > Review of attachment 233165 [details] [review]: > > ::: panels/info/cc-info-panel.c > @@ +437,2 @@ > static gboolean > +get_current_is_fallback (CcInfoPanel *self) > > What's that? Remove it. Probably an incorrect rebase. Removed. > > ::: shell/cc-hostname-entry.c > @@ +205,3 @@ > + > +static void > +cc_hostname_entry_set_property (GObject *object, > > Remove the unused get/set property calls. > > @@ +237,3 @@ > +cc_hostname_entry_finalize (GObject *object) > +{ > + G_OBJECT_CLASS (cc_hostname_entry_parent_class)->finalize (object); > > Remove the chained up finalize function. Done.
Created attachment 233431 [details] [review] Add CcHostnameEntry widget and use it in the info panel Actually update patch.
Created attachment 233455 [details] [review] Add initial implementation of the new Sharing panel (In reply to comment #9) > Review of attachment 233166 [details] [review]: > > > ::: panels/sharing/cc-sharing-panel.c > @@ +63,3 @@ > + { > + gtk_widget_destroy (priv->bluetooth_sharing_dialog); > + priv->bluetooth_sharing_dialog = NULL; > > g_clear_object() Using g_clear_object() causes a segmentation fault. I think gtk_widget_destroy() is required to break any other references to the widget. > > ::: panels/sharing/vino-preferences.c > @@ +104,3 @@ > + string = g_value_get_string (value); > + > +#if 0 > > Implement? The keyring code doesn't actually appear to be implemented in Vino itself (the function calls always return NULL or FALSE), so I've removed the code instead.
Review of attachment 233431 [details] [review]: Looks good.
Review of attachment 233455 [details] [review]: Looks good overall, minus those small nits. Please commit when you've fixed those, and we can take it from there. ::: panels/sharing/cc-sharing-panel.c @@ +198,3 @@ + + if (active) + g_value_set_string (target_value, _("On")); Use C_() here. @@ +200,3 @@ + g_value_set_string (target_value, _("On")); + else + g_value_set_string (target_value, _("Off")); Ditto. @@ +242,3 @@ + gpointer user_data) +{ + if (g_str_equal (g_variant_get_string (variant, NULL), "bonded")) gboolean bonded; bonded = g_str_equal (g_variant_get_string (variant, NULL), "bonded"); g_value_set_boolean (value, bonded); ::: panels/sharing/gnome-sharing-panel.desktop.in.in @@ +11,3 @@ +X-GNOME-Settings-Panel=sharing +# Translators: those are keywords for the sharing control-center panel +_Keywords=Share, sharing, ssh, host, name, remote, desktop, bluetooth, obex Hmm, that wasn't supposed to be a literal list ;) That would be: _Keywords=share;sharing;ssh;host;name;remote;desktop;bluetooth;obex;
Comment on attachment 233455 [details] [review] Add initial implementation of the new Sharing panel Committed with changes suggested above.
It's all committed, so closing. There's a bunch of separate bugs against the Sharing component already.