GNOME Bugzilla – Bug 659207
Some preferences are obscure or obsolete with gnome 3
Last modified: 2011-09-26 10:13:18 UTC
With the streamlining of the chat experience in 3.2, I find some of Empathy's preferences obscure or obsolete now. I think these here could be removed completely General -> Behavior - "Display incoming events in the notification area" - shouldn't this be automatic and tied to the Notifications switch? Does this even change something? - "Automatically connect on startup" - I think this is automatically remembered from the user menu status now? Notifications - this should probably go away completely?
- "Display incoming events in the notification area" - (In reply to comment #0) > With the streamlining of the chat experience in 3.2, I find some of Empathy's > preferences obscure or obsolete now. I think these here could be removed > completely Indeed, some preferences don't make sense in the GNOME Shell world. > General -> Behavior > - "Display incoming events in the notification area" - shouldn't this be > automatic and tied to the Notifications switch? Does this even change > something? Right, this one shouldn't appear when using the Shell as we don't display the status icon when using the Shell any more. > - "Automatically connect on startup" - I think this is automatically remembered > from the user menu status now? It's not (bug #659021), but I agree that it makes less sense now that the we don't change presence to offline when exiting if we are using the Shell. On the other hand, one could want to connect when starting Empathy without having to manually set his presence to online. > Notifications - this should probably go away completely? Yeah the Shell discard those notifications any way so we can hide it too. I'd go for a "Hide if Shell is running" approach rather than removing them competely as I'd prefer to avoid breaking non GNOME 3 desktop for now.
(In reply to comment #1) > > - "Automatically connect on startup" - I think this is automatically remembered > > from the user menu status now? > > It's not (bug #659021), but I agree that it makes less sense now that the we > don't change presence to offline when exiting if we are using the Shell. > On the other hand, one could want to connect when starting Empathy without > having to manually set his presence to online. Yes, I believe a good behavior here could be: Empathy should connect automatically by default when opening, setting the last used presence. There's pretty much nothing we could do with the contact list without going online anyway, no? > I'd go for a "Hide if Shell is running" approach rather than removing them > competely as I'd prefer to avoid breaking non GNOME 3 desktop for now. I think you can check for the org.gnome.Shell bus name.
(In reply to comment #2) > (In reply to comment #1) > > > > - "Automatically connect on startup" - I think this is automatically remembered > > > from the user menu status now? > > > > It's not (bug #659021), but I agree that it makes less sense now that the we > > don't change presence to offline when exiting if we are using the Shell. > > On the other hand, one could want to connect when starting Empathy without > > having to manually set his presence to online. > > Yes, I believe a good behavior here could be: Empathy should connect > automatically by default when opening, setting the last used presence. There's > pretty much nothing we could do with the contact list without going online > anyway, no? Watching your logs, but yeah that's basically it I think. > > I'd go for a "Hide if Shell is running" approach rather than removing them > > competely as I'd prefer to avoid breaking non GNOME 3 desktop for now. > > I think you can check for the org.gnome.Shell bus name. Yeah we are already doing that to disable the status icon when using the Shell.
Since all the widgets that must be hidden if Shell is running are located in Preferences, could we pass shell_running when creating the preferences window?
Yeah, sounds like a sane way to do it.
Created attachment 197136 [details] [review] Added new property to MainWindow and pass it to Preferences when preferences_new() I do not know if this is a valid way to achive what we want. I have added a new property to MainWindow with a value inherited from EmpathyApp and set asynchronously. Then, since MainWindow is responsible for creating the preferences window using preferences_new(), it passes the value of that property to this constructor, and it set the visible property accordingly.
Review of attachment 197136 [details] [review]: Cool! Conceptually good but some cleanup here and there. ::: src/empathy-main-window.c @@ +2144,3 @@ + + pspec = g_param_spec_boolean ("shell-running", + "Whether the Shell is running or not", the nick should be something like "Shell running" and this line should be the blurb. @@ +2147,3 @@ + "Get the Shell running status", + FALSE, + G_PARAM_READWRITE); You should set G_PARAM_STATIC_STRINGS as well (pretty useless, but good practice). ::: src/empathy-preferences.c @@ +1285,2 @@ GtkWidget * +empathy_preferences_new (GtkWindow *parent, gboolean shell_running) one arg per line. @@ +1299,3 @@ + priv = GET_PRIV (self); + if (shell_running) { Please add a comment explaining these options are meaningless when using GNOME Shell. @@ +1300,3 @@ + priv = GET_PRIV (self); + if (shell_running) { + gtk_widget_hide (priv->checkbutton_events_notif_area); As said above, you should hide the "notifications" tab as well. ::: src/empathy-preferences.h @@ +65,3 @@ GType empathy_preferences_get_type (void); +GtkWidget *empathy_preferences_new (GtkWindow *parent, gboolean shell_running); one arg per line please. ::: src/empathy.c @@ +292,3 @@ + g_value_init (&shell_running_val, G_TYPE_BOOLEAN); + g_value_set_boolean (&shell_running_val, TRUE); + g_object_set_property (G_OBJECT (self->window), You can use g_object_set() to avoid having to create a GValue. But tbh, I think I'd prefer empathy_main_window_set_shell_running ().
Created attachment 197195 [details] [review] Fixed patch New patch with modifications pointed out in the last review committed. Probably there are still some errors though ;) It seems to work in Fedora15 in both modes: shell and fallback.
Review of attachment 197195 [details] [review]: Looking code! Thanks for your work on this. ::: src/empathy-main-window.c @@ +2077,3 @@ + gboolean shell_running) +{ + g_object_set (G_OBJECT (window), "shell-running", shell_running, NULL); No need to do that on yourself. Just: if (priv->shell_running == shell_running) return; priv->shell_running = shell_running; g_object_notify (window, "shell-running");
Created attachment 197218 [details] [review] Fixed empathy_main_window_set_shell_running () I just modified what you pointed out. I did not realize that was MainWindow code and therefore can access private members directly ;)
Review of attachment 197218 [details] [review]: Looks good! I'll merge once we branch.
Merged to master; thanks!