GNOME Bugzilla – Bug 756323
Hide focus-indication when interacting with room list using the pointer
Last modified: 2015-11-04 00:49:17 UTC
Created attachment 312991 [details] screenshot of the polari room having focus indication. If I click on a room in the the roomlist sidebar, then the GTK+'s focus-indication appears (the dashed pattern around the label). We already mark the chosen room blue when clicking on it, so there is no need to also show focus-indication. In Nautilus the dashes are not visible until you begin to press tab or use arrow keys which makes a lot of sense.
I played around with reproducing this bug, and it seems to only occur if you interact with the menus using keys first. For example, starting up Polari, I can't produce any dashes with my cursor if I click on rooms. However, if I hit tab to cycle through something, then both the keys and the cursor will invoke focus-indication for the rest of the time Polari is open.
After chatting in IRC, I built the latest version of Nautilus (which, as Florian pointed out, uses GtkListBox now). It actually exhibits the same behavior in 3.18. Not sure if it's something that should be looked at more generally.
(In reply to Cody Welsh from comment #2) > It actually exhibits the same behavior in 3.18. Mmh, I haven't seen it in nautilus, nor have I looked at its code (yet). However I suspect that it's moving the key focus when displaying a folder. > Not sure if it's something that should be looked at more > generally. Maybe. There are precedents for GtkWidgets to have a :focus-on-click property, maybe it's worth adding something similar to GtkListBox/GtkListBoxRow.
Yeah, I've been trying to inspect the code a bit; my C is rusty, though ;) > There are precedents for GtkWidgets to have a :focus-on-click property, maybe > it's worth adding something similar to GtkListBox/GtkListBoxRow. Is it worth adding custom functionality for this bug, if we're considering putting a :focus-on-click property in the parent row element?
Actually, I think it's probably worth just comparing the functionality of GtkTreeView versus GtkListBox. I'm not entirely clear as to what the difference is, yet, with regards to keyboard and cursor interaction, however.
Looking in GtkWidget docs, you can see that the useful properties one might use are read-only: https://developer.gnome.org/gtk3/stable/GtkWidget.html#GtkWidget.style-properties However, if we want to, we can apply a different CSS style (as is suggested there) - E.G., outline-style: hidden - when we focus from the mouse instead. That would still need for us to listen for keycodes, which we already seem to do as well in the RoomRow definition (although, it seems unrelated): 92 _onKeyPress: function(w, event) { 93 let [, keyval] = event.get_keyval(); 94 let [, mods] = event.get_state(); 95 if (keyval != Gdk.KEY_Menu && 96 !(keyval == Gdk.KEY_F10 && 97 mods & Gdk.ModifierType.SHIFT_MASK)) 98 return Gdk.EVENT_PROPAGATE; 99 100 this._popover.show(); 101 102 return Gdk.EVENT_STOP; 103 }, So, I guess the question is, "Do we want to add functionality right now that has a non-zero chance of being deprecated soon?"
This is not something that applications are expected to manage manually. GTK+ should do this for you automatically.
(In reply to Cody Welsh from comment #6) > Looking in GtkWidget docs, you can see that the useful properties one might > use are read-only ... No. The only acceptable way (IMHO) to implement a focus-on-click == false policy in the app itself is to move the key focus elsewhere, not to mess with GTK+'s focus indication. Well, apart from an actual GtkListBoxRow:focus-on-click property of course. Not sure this is really important enough to be honest, so I think we should make this bug dependent on getting this functionality in GTK+ and just close as WONTFIX if we don't. Matthias, how do you feel about such a property? Worth writing a patch for that or are the existing property a relict pattern we don't want spreading to newer widgets?
Oh, now I see what you meant by moving the key focus. If that's the only option, it does seem a little "hacky" for this (in my uneducated opinion), especially if we're going to change it later.
(In reply to Florian Müllner from comment #8) > Matthias, how do you feel about such a property? Worth writing a patch for > that or are the existing property a relict pattern we don't want spreading > to newer widgets? We have this property duplicated in 3 widgets already, maybe it is time to move it to GtkWidget and make it generally available.
Created attachment 314347 [details] [review] roomList: Don't focus room rows when clicked When switching to a room by clicking the corresponding row, it is much more likely that the user wants to take part in the room's conversation than using keynav to navigate the room list. So don't steal focus from the entry by setting GtkWidget:focus-on-click appropriately.
Review of attachment 314347 [details] [review]: Tried to test it by applying the patches in bug 757269 and this patch and it seems to work nicely. I do see this output in the terminal though: [bastian@archxps gtk+]$ jhbuild run polari Gtk-Message: Failed to load module "canberra-gtk-module" Gtk-Message: Failed to load module "canberra-gtk-module" (org.gnome.Polari:30665): Gtk-WARNING **: Allocating size to GtkHeaderBar 0x82c350 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate? (org.gnome.Polari:30665): tp-logger-CRITICAL **: log_store_pidgin_set_basedir: assertion 'self->priv->basedir == NULL' failed (org.gnome.Polari:30665): Gtk-CRITICAL **: gtk_widget_draw: assertion '!widget->priv->alloc_needed' failed (org.gnome.Polari:30665): Gtk-CRITICAL **: gtk_widget_draw: assertion '!widget->priv->alloc_needed' failed (org.gnome.Polari:30665): Gtk-CRITICAL **: gtk_widget_draw: assertion '!widget->priv->alloc_needed' failed (org.gnome.Polari:30665): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width 0 and height -4 (org.gnome.Polari:30665): Gtk-WARNING **: gtk_widget_size_allocate(): attempt to allocate widget with width -15 and height -3
(In reply to Bastian Ilsø from comment #12) > I do see this output in the terminal though There have been various allocation-related changes on gtk master (https://git.gnome.org/browse/gtk+/commit?id=6866d1c06e338 for instance). It's something we need to look into of course, but unrelated to the patches here and the corresponding gtk+ bug.
Attachment 314347 [details] pushed as 56f0f9a - roomList: Don't focus room rows when clicked