GNOME Bugzilla – Bug 650699
add a way to hide panels
Last modified: 2018-04-17 15:00:24 UTC
Created attachment 188256 [details] [review] patch This might be necessary for panels that are only good for specific types of hw. Here is a patch that makes this possible, by letting the panels implement a should_be_visible() vfunc.
I like this idea a lot, although doesn't a vfunc mean that we have to load all the plugin deps at startup rather than lazy load them? Maybe we do this already...
We already load all those plugins, we just don't activate them. What I don't really like is the sync functions. What devices would this be used for, apart from tablets?
Don't really know of examples, that's why I decided to park this patch here until it becomes relevant...
wacom tablets come to mind as an example
(In reply to comment #4) > wacom tablets come to mind as an example comment 2 says "apart from tablets". It's not a good example as tablets can be Bluetooth ones (eg. configured but not connected) and we'd need the visibility of the panel to be instantaneous.
*** Bug 659617 has been marked as a duplicate of this bug. ***
*** Bug 661097 has been marked as a duplicate of this bug. ***
Another example that was brought up is - bluetooth, if your system doesn't have a bt adapter - keyboard and mouse, if this is a tablet without those
A small idea: we could arrange things so that panels still show up in the search results, even when they are 'hidden'. Then a search for 'wacom' would still get you to the tablet panel
(In reply to comment #9) That was my proposal. I would still indicate them not being detected/available using 50% opacity on the icon.
I don't think that we should be hiding the Bluetooth panel. Maybe we should give a way for system administrators, or system integrators to hide it, but the fact is that there's barely any (laptop) machines without Bluetooth right now. There's also the fact that it's hard to query for it quickly, without blocking the UI. I would always show it. Keyboard and mouse could be hidden, Wacom certainly, we would also hide either the "User Accounts", or the "User Account" panel (bug 681762) based on a lockdown value. In Matthias' patch, we should have 3 different returns for cc_panel_class_should_be_visible(): - show - hide, but show in the search results - always hide We might also want the value to change based on hotplug, so having that return value as a property would make it easier to hook up to the models we use.
*** Bug 689202 has been marked as a duplicate of this bug. ***
*** Bug 689201 has been marked as a duplicate of this bug. ***
*** Bug 690657 has been marked as a duplicate of this bug. ***
Created attachment 242792 [details] [review] Shell: check OnlyShowIn from desktop file to determine if we whould display icons. This is essentially our solution to bug 690657, but that was marked a dupe of this so will post here. It does however provide a nice easy way for distro's to hide irrellevant panels based on desktop, which was the previous behaviour before the gmenu code was removed.
Comment on attachment 242792 [details] [review] Shell: check OnlyShowIn from desktop file to determine if we whould display icons. That's really not what this bug is about. Furthermore, if somebody is patching the control-center to add new panels, they can also do this in their own code.
Since the control center shell changed to use a sidebar, it seems like we should revisit this bug. I'd always imagined that one of the objectives of the sidebar design was to make it easier to hide some settings panels when they're not relevant. Possibilities include: * Wi-Fi * Bluetooth * Keyboard * Mouse & Touchpad * Wacom Tablet
If we decide to go for this feature, we should at least offer a gsetting for "show-all-panels", for use-cases where this behavior is unwanted.
(In reply to Felipe Borges from comment #18) > If we decide to go for this feature, we should at least offer a gsetting for > "show-all-panels", for use-cases where this behavior is unwanted. No need. The search (both in the control-center shell and in gnome-shell) would still show the panels.
Today's experience for people on desktops or other systems without wifi is pretty sub-optimal: you start the control center and it chooses the first item on the list which happens to be... Wi-Fi! So you get a default screen with a big question mark in it, saying "No Wi-Fi Adapter Found". I also have no Bluetooth adapter. And adding insult to injury, important stuff is not visible in the menu without scrolling (bug #789372 and bug #786810) to make room for these. It could be that I would plug in a USB wifi or bluetooth adapter so I guess the hardware could come into existence sometime in the future.
(In reply to Allan Day from comment #17) > Since the control center shell changed to use a sidebar, it seems like we > should revisit this bug. I'd always imagined that one of the objectives of > the sidebar design was to make it easier to hide some settings panels when > they're not relevant. Possibilities include: > > * Wi-Fi > * Bluetooth > * Keyboard > * Mouse & Touchpad > * Wacom Tablet Maybe Network panel as well when NetworkManager is not running(e.g. someone uses wicked to manage their netowrk).
(In reply to Bastien Nocera from comment #2) > We already load all those plugins, we just don't activate them. I'm trying to implement this feature recently, and find out that it seems that this is not true anymore with commit 0139f684162a10dfa159313552ff96b08f359cd5. So adding a should_be_visible() vfunc won't work. Correct me if I'm wrong.
(In reply to Jonathan Kang from comment #22) > (In reply to Bastien Nocera from comment #2) > > We already load all those plugins, we just don't activate them. > > I'm trying to implement this feature recently, and find out that it seems > that > this is not true anymore with commit > 0139f684162a10dfa159313552ff96b08f359cd5. > So adding a should_be_visible() vfunc won't work. > > Correct me if I'm wrong. It still works as it did then. The vfunc should be on the class, not the instance, as the instance is only created when the panel is loaded.
Created attachment 366702 [details] [review] shell: Add _get_visibility() vfunc to hide panels _get_visibility() vfunc is added to CcPanelClass to enable the ability to hide panels if necessary.
Review of attachment 366702 [details] [review]: That doesn't seem to allow panels to appear as services/devices become available, just checked once at startup. I think the class member should probably be for a class signal (I'm not sure whether GObject supports those though. It would also be good to: - split off the enum declaration in a separate patch - split off the API addition for panels from the use of that API in the shell itself And, in a separate patch, implementation for at least one panel. For example, whether or not to show the Wi-Fi panel, and making sure that the panel appears when unblocking hardware rfkill on it, or plugging in a Wi-Fi dongle if the machine you're testing this on doesn't have a Wi-Fi adapter. Or showing/hiding the Wacom panel, which might be easier to implement. ::: shell/cc-panel.h @@ +57,3 @@ + * @CC_PANEL_VISIBILITY_HIDE: + * + * Visibility of panels, to devide whether a panel should be visible or not. divide? decide?
(In reply to Bastien Nocera from comment #25) > Review of attachment 366702 [details] [review] [review]: > > That doesn't seem to allow panels to appear as services/devices become > available, just checked once at startup. > > I think the class member should probably be for a class signal (I'm not sure > whether GObject supports those though. We can use signals(show/hide that panel) emitted by each panel, and maybe connect to some callbacks in cc-panel-list.c where we can add/remove panels from the list. I'm not quite sure if this is doable. But I'll spend some time on it. > > It would also be good to: > - split off the enum declaration in a separate patch > - split off the API addition for panels from the use of that API in the > shell itself Sure. > > And, in a separate patch, implementation for at least one panel. For > example, whether or not to show the Wi-Fi panel, and making sure that the > panel appears when unblocking hardware rfkill on it, or plugging in a Wi-Fi > dongle if the machine you're testing this on doesn't have a Wi-Fi adapter. > Or showing/hiding the Wacom panel, which might be easier to implement. Bluetooth works for me as I have a PC without bluetooth adapter and a USB bluetooth adapter. So I'll try to implement this. > > ::: shell/cc-panel.h > @@ +57,3 @@ > + * @CC_PANEL_VISIBILITY_HIDE: > + * > + * Visibility of panels, to devide whether a panel should be visible or not. > > divide? decide? decide. It was a typo. Sorry about that.
Created attachment 367002 [details] [review] shell: Add CcPanelVisibility enum
Created attachment 367003 [details] [review] shell: Add _get_visibility() vfunc to hide panels
I attached the first two patches here for review. The implementation in the shell part is still ongoing. I'll attach it when it's finished.
Review of attachment 367002 [details] [review]: It's simple enough, but I'll wait to have a more in depth look when the whole patchset is ready.
Review of attachment 367003 [details] [review]: Same as the first one
Created attachment 367561 [details] [review] shell: hide panels when control center starts Sorry for being this long since last two patches. I'm having trouble implementing this feature that showing/hiding panels when related service/device goes up/down. This patch just hides panels when control center starts. Basically the problem I met is not knowing a way to connect signals(service/device goes up/down) from various panels to callbacks in CcPanelList where we can show/hide that panel from the list. For example, if users haven't clicked the Bluetooth panel yet, no instance of CcBluetoothPanel will be created. So when users plug/unplug a bluetooth dongle, no instance is able to emit such a signal for more actions that need to be done. Maybe we can use some utility functions/codes for such a thing? What do you think of this idea? I want raise this issue here for some discussions. Thanks
(In reply to Jonathan Kang from comment #32) > Created attachment 367561 [details] [review] [review] > shell: hide panels when control center starts > > Sorry for being this long since last two patches. I'm having trouble > implementing > this feature that showing/hiding panels when related service/device goes > up/down. > > This patch just hides panels when control center starts. > > Basically the problem I met is not knowing a way to connect > signals(service/device goes up/down) from various panels to callbacks in > CcPanelList where we can show/hide that panel from the list. > > For example, if users haven't clicked the Bluetooth panel yet, no instance > of CcBluetoothPanel will be created. So when users plug/unplug a bluetooth > dongle, no instance is able to emit such a signal for more actions that need > to be done. > > Maybe we can use some utility functions/codes for such a thing? What do you > think of this idea? I want raise this issue here for some discussions. You need to implement this at the class-level, not at the instance level. Either that, or come up with a companion object for each panel to implement this.
Created attachment 367893 [details] [review] shell: show/hide bluetooth panel when needed Hi, this is still a WIP patch. I'm attaching this to show the way I want to implement this: 1. Add needed object to emit signals when devices/services go up/down. In this patch, add "GDBusProxy *bluetooth_rfkill;" in _CcPanelList to indicate when bluetooth device is available/unavailable. 2. Connect related signal to a callback function in cc-panel-list.c to show/hide the bluetooth panel. Comments are welcome.
The discussion can continue here, where an implementation is already available: https://gitlab.gnome.org/GNOME/gnome-control-center/merge_requests/29