GNOME Bugzilla – Bug 323028
Scroll GtkMenu on click (gtk-touchscreen-mode)
Last modified: 2011-02-04 16:10:23 UTC
For touchscreen (stylus) environments, GtkMenu scrolling is broken since it relies on the user hovering the scroll arrows. Touchscreens need GtkMenus which scroll on click instead.
Created attachment 55522 [details] [review] Patch implementing menu scrolling on click Attached patch adds a new property "gtk-touchscreen-mode" to GtkSettings and uses that in GtkMenu to scoll on click instead on hovering the arrows. The patch also contains a new style property for GtkMenu, "double-arrows" which, when enabled, causes both arrows to be visible as soon as the menu is too large. The two features are in one patch because they both fiddle with the new private members "upper_arrow_state" and "lower_arrow_state" and should preferrably be comitted together if accepted, tho it's of course feasible to change the patch to allow click-in-scroll without the double arrows stuff.
It would be great if we could get a fix for http://bugzilla.gnome.org/show_bug.cgi?id=129463 if you are fiddling with the scroll menu visuals anyway...
Hm, that would be about the same stuff as the "double-arrows" property included in the patch. Shouldn't we just always show both arrows, which would also simplify the code quite a bit? It would also be a better indication of the scrollability of the menu (regardless of any empty areas).
Created attachment 55556 [details] [review] Previous patch became unapplyable after adding the horizontal-padding property
Created attachment 55633 [details] [review] Updated patch fixes minor glitch with scroll arrow sensitivity
There are some problems with option menus here. I had the touchscreen-mode and double-arrows turned on. When moving the testgtk menu example close to the bottom of the screen, it comes up with just one arrow (despite double-arrows being turned on and working for regular menus). When I use that arrow to scroll the menu all the way in, the other arrow appears. After clicking on the other arrow I'm left with a stuck grab, and the menu won't go away.
I'm trying to fix this at the moment.
Created attachment 55707 [details] [review] Enhanced patch fixes the mentioned issues and some more
The patch seems to work fine for me now. I notice it introduces the touchscreen-mode setting that has been discussed in several other bugs and wiki pages. I think we need to add a "Touchscreen" section to the docs which collects all the differences (since we are going to have more, right ?). Also, should that setting be exposed as an X setting ? I guess probably not, since it is really a setting thats fixed by the choice of desktop environment, not something you want to change inside the environment. Ie you would turn it on in a typical maemo environment, and you would turn it off in a typical Gnome environment, but you wouldn't want to tweak it in a running Gnome session. Like the button order setting.
I guess a dualhead setup with one normal and one touchscreen is worth being supported. Don't know if that involves the need for an X setting.
Hmm, GtkSettings are per-screen objects, but the only way to set them per-screen seems to be via xsettings. Unless you want to set the settings manually in the application.
Alex, would you mind having a look at that patch too? You did the scrolling thing, and I probably didn't 100% understand any aspect of it.
Adding Søren too, since he is the most recent GtkMenu hacker.
Wow, i last touched that years and years ago. I'm not sure I'm much help with this.
I need a working cvs HEAD before I can try this patch. See bug 324461.
Created attachment 56152 [details] [review] Updated patch (GtkSettings changes gave conflicts)
Created attachment 56158 [details] [review] Next one (previous patch triggered an assertion in GtkSettings)
I don't have a touchscreen, so I could be wrong about some of the behavior stuff below. 1. Entering the lower arrow causes a tiny scroll, that will make the upper arrow appaer. I don't think enter/leaving should ever cause this to happen in touchscreen mode. I haven't debugged it, but my guess would be that the leavenotify handler doesn't check for touchscreen. 2. A normal click doesn't cause a scroll, which I think it should. 3. On a touchscreen that doesn't generate motion events, 1 & 2 combined will have the effect that a click causes a small scroll, because the click will cause a leave, then a mousepress. But relying on this side effect is a bad hack in my opinion. In fact, it looks to me like repeated clicking on an arrow will only cause one scroll to happen. 4. If you keep the mouse button held down, the menu will scroll quickly, but then when the arrow disappears, the last item will be activated. I suppose with the double-arrows setting, this is not a problem. This makes touchscreen-mode buggy without the double-arrow setting, so I think double-arrow should probably just be part of touchscreen-mode instead of its own setting. (I know I said on IRC that double arrows might be useful as default, but after trying it out, I don't think so anyore). Looking at the code, there are some issues - Would it be possible to move all the scroll handling to gtk_menu_handle_scrolling(). Ie., instead of checking for touchscreen and calling gtk_menu_handle_scrolling() based on that, just call it unconditionally, then do the checks in gtk_menu_handle_scrolling(). Conceptually, the function would always be 'safe' to call; it would figure out by itself what to do. - Even if that's not possible, the new code blocks in press() and release() should certainly be facotored out in a function. - Two minor style issues: - We don't put spaces after !'s. - "popped", not "poped"
1. touchscreen-mode essentially means "no motion events", nothing else. So I don't think it is a problem that entering an arrow makes a partly-obscured item fully visible 2. wasn't there in earlier versions of the patch, it somehow broke. Will look into that. 3. that side effect looks like it's caused by the mis-behavior in 2. 4. yes they should probably always be used together. However i think that double-arrows can be very useful without touchscreen mode, esp with combo box menus, which often pop up with a scroll offset and a blank area. On the other hand the behavior is consistent with the following: Scroll a bit, then (while keeping mouse pressed), move to an item and release --> the item is selected. The same applies to the last item that appears under the mouse when the arrow disappears, it gets selected on button release. Will come up with a new patch, fixing (2) and trying to address the code issues you mentioned.
Created attachment 56312 [details] [review] Some issues fixed New patch merges changes to gtksettings.c and addresses two menu issues: - no space after '!' - click now scrolls reliably Will address the remaining issues later, just want to upload an intermediate patch that at cleanly applies.
Created attachment 59411 [details] [review] Next patch iteration This version fixes all the issues mentioned above. Unfortunately it introduces uglyness (priv->ignore_button_release) which is used to prevent the menu item that is uncovered when hiding a scroll arrow from being activated. I'm open to suggestions how to solve this more nicely (if anything in gtkmenu can be called nice ;-)
hm, are there any significant missing/broken things about the patch still, or can this go in in its current form? i didn't find anything obviously wrong about it, though, it's been a time since i hacked gtkmenu code. a few comments nevertheless: - gtk_menu_do_timeout_scroll(): there should be a comment next to ignore_button_release = TRUE telling the story about hiding arrows. - i tend to favour to default to TRUE for ::double-arrows, just like mitch. accompanied by the sensitivity settings for the scrolling arrows, this resembles scrollbar behaviour, and we have enough screen space for two arrows anyway. - regardless of the default for ::double-arrows, to also fix #129463 (as mentioned in Comment #2), the patch logic should be changed to always show both scrolling arrows if (::double-arrows || push_in) is TRUE, where 'push_in' denotes the boolean returned from position_func(). this change can be done once mitch's patch has been applied, and if neccessary can be discussed in #129463 before applying it (i.e. shouldn't block mitch's last patch from being applied).
Re comment #21, I don't think ignore_button_release is worse than ignore_enter, and I agree with Tim's comments. I'll ask Soeren if he wants to review the new patch again, otherwise I think we should get this in cvs with the changes proposed by Tim, and fix up any remaining issues afterwards, though I don't see any at the moment.
Created attachment 59845 [details] [review] Fixes #129463 too New patch shows both arrows unconditionally for popup menus which are pushed in, thus also fixes bug #129463.
thanks mitch. the new patch can go in as far as i'm concerned.
Soren said he is fine with committing this if everybody else thinks its fine, so go ahead.
Fixed in CVS: 2006-02-22 Michael Natterer <mitch@imendio.com> * gtk/gtksettings.c: added boolean property gtk-touchscreen-mode, which essentially means "there are no motion notify events", so widgets can't use the pointer hovering them for anything. * gtk/gtkmenu.c: if gtk-touchscreen-mode is TRUE, scroll menus when clicking the scroll arrows, since hovering goes undetected. Fixes bug #323028. Added boolean style property "double-arrows" which always makes both scroll arrows visible when the menu is too long. For pushed-in popup menus, both arrows are always shown (regardless of double-arrows), in order to fix user confusion about the blank area. Fixes bug #129463.