After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 687772 - Implement the Sharing panel designs
Implement the Sharing panel designs
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Sharing
3.7.x
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 636206 658498 (view as bug list)
Depends on: 687780
Blocks:
 
 
Reported: 2012-11-06 15:48 UTC by Allan Day
Modified: 2013-01-16 10:02 UTC
See Also:
GNOME target: ---
GNOME version: 3.7/3.8


Attachments
Add CcHostnameEntry widget and use it in the info panel (38.44 KB, patch)
2013-01-10 18:01 UTC, Thomas Wood
reviewed Details | Review
Add initial implementation of the new Sharing panel (101.27 KB, patch)
2013-01-10 18:02 UTC, Thomas Wood
needs-work Details | Review
Updated patch for the initial implementation of the sharing panel (110.68 KB, patch)
2013-01-11 17:45 UTC, Thomas Wood
none Details | Review
Add CcHostnameEntry widget and use it in the info panel (38.44 KB, patch)
2013-01-14 10:35 UTC, Thomas Wood
none Details | Review
Add CcHostnameEntry widget and use it in the info panel (36.23 KB, patch)
2013-01-14 10:37 UTC, Thomas Wood
committed Details | Review
Add initial implementation of the new Sharing panel (114.59 KB, patch)
2013-01-14 16:34 UTC, Thomas Wood
committed Details | Review

Description Allan Day 2012-11-06 15:48: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
Comment 1 Allan Day 2012-11-06 15:48:51 UTC
*** Bug 636206 has been marked as a duplicate of this bug. ***
Comment 2 Allan Day 2012-11-06 15:49:01 UTC
*** Bug 658498 has been marked as a duplicate of this bug. ***
Comment 3 el 2012-11-06 16:15:20 UTC
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).
Comment 4 Bastien Nocera 2012-11-12 10:55:51 UTC
(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.
Comment 5 el 2012-11-12 14:08:14 UTC
Sorry my comment must have ended up on the wrong bug report.
Comment 6 Thomas Wood 2013-01-10 18:01:59 UTC
Created attachment 233165 [details] [review]
Add CcHostnameEntry widget and use it in the info panel
Comment 7 Thomas Wood 2013-01-10 18:02:03 UTC
Created attachment 233166 [details] [review]
Add initial implementation of the new Sharing panel
Comment 8 Bastien Nocera 2013-01-11 07:07:15 UTC
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.
Comment 9 Bastien Nocera 2013-01-11 07:20:37 UTC
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?
Comment 10 Thomas Wood 2013-01-11 17:45:08 UTC
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.
Comment 11 Thomas Wood 2013-01-14 10:35:52 UTC
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.
Comment 12 Thomas Wood 2013-01-14 10:37:59 UTC
Created attachment 233431 [details] [review]
Add CcHostnameEntry widget and use it in the info panel

Actually update patch.
Comment 13 Thomas Wood 2013-01-14 16:34:13 UTC
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.
Comment 14 Bastien Nocera 2013-01-14 17:22:43 UTC
Review of attachment 233431 [details] [review]:

Looks good.
Comment 15 Bastien Nocera 2013-01-14 17:30:44 UTC
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 16 Thomas Wood 2013-01-14 18:28:48 UTC
Comment on attachment 233455 [details] [review]
Add initial implementation of the new Sharing panel

Committed with changes suggested above.
Comment 17 Bastien Nocera 2013-01-16 10:02:24 UTC
It's all committed, so closing. There's a bunch of separate bugs against the Sharing component already.