GNOME Bugzilla – Bug 734285
Change defaults to reduce boilerplate in code that uses gestures
Last modified: 2014-08-15 11:56:33 UTC
I'd like to suggest two changes to the gesture api: 1) set the default propagation phase to "bubble". The current default is "none" which isn't needed in a lot of common cases. 2) set the default for touch-only in single gestures to "false" and the default button to 1. swiping or long pressing with a mouse should frequently behave the same way as swiping or long pressing with a finger in my opinion. By making these changes, users of the gesture api will frequently have less code to write.
also 3) gestures should automatically update the event masks of the widgets they're set on
sounds plausible to me - Carlos ?
If we want to do this, we'll have to land it soon.
Good points Ray, Seeing how the API has evolved I think it makes sense to have such defaults, the initial/current ones being initially thought out as helpers for cases where GtkWidgetClass event handlers were more resilient, but it doesn't really look like a good thing to stick defaults to. > I'd like to suggest two changes to the gesture api: > > 1) set the default propagation phase to "bubble". The current default is > "none" which isn't needed in a lot of common cases. > > 2) set the default for touch-only in single gestures to "false" and the default > button to 1. swiping or long pressing with a mouse should frequently behave > the same way as swiping or long pressing with a finger in my opinion. I'm attaching a bunch of patches for these ones, with the default value changes, and fixes in places where the older default was assumed. I'm not attaching here any pure cleanup commits yet to avoid cluttering the bug. I think I had the basics covered by following the next hints: - gtk_event_controller_handle_event() is only called in gtkdnd.c and gtkwindow.c, those places need to set GTK_PHASE_NONE. - callers of gtk_gesture_single_get_current_button() will need gtk_gesture_single_set_button(gesture,0) now. - pulling from memory, the only behavior that is exclusively meant for touchscreens and implemented through GtkGesture is scrolledwindow kinetic scrolling and spinbutton swipes. (In reply to comment #1) > also 3) gestures should automatically update the event masks of the widgets > they're set on I think this is already covered by the patch in https://bugzilla.gnome.org/show_bug.cgi?id=734357#c3
Created attachment 283184 [details] [review] eventcontroller: Default to GTK_PHASE_BUBBLE This is in practice the most common value, so make that the default
Created attachment 283185 [details] [review] dnd: Set drag gesture propagation phase to GTK_PHASE_NONE explicitly This used to rely on the default phase value.
Created attachment 283186 [details] [review] window: Set multipress gesture phase to be GTK_PHASE_NONE explicitly This used to rely on the default value, which has changed to a saner default.
Created attachment 283187 [details] [review] gesturesingle: Change default values of button and touch-only properties This now defaults to sane values on mice, so touch-only is set to FALSE, and button is set to GDK_BUTTON_PRIMARY;
Created attachment 283188 [details] [review] colorplane: Update to new GtkGestureSingle/GtkEventController defaults GtkGestureSingle::button is set to 0, as multiple buttons are managed by the same gesture. Also avoid some extra lines of code setting what nowadays are default values.
Created attachment 283189 [details] [review] colorswatch: Update to new GtkGestureSingle/GtkEventController defaults GtkGestureSingle::button is set to 0 on the multipress gesture, as several buttons are managed by that gesture. Also avoid some extra lines of code setting what nowadays are default values.
Created attachment 283190 [details] [review] dnd: Set explicitly GtkGestureSingle::button to 0 on the drag gesture This gesture handles drags done with any button, so unset the current button=1 default.
Created attachment 283191 [details] [review] entry: Update to new GtkGestureSingle/GtkEventController defaults GtkGestureSingle::button is set to 0 on the drag/multipress gestures, as several buttons are managed by these gestures. Also, avoid some extra lines of code setting what nowadays are default values.
Created attachment 283192 [details] [review] label: Update to new GtkGestureSingle/GtkEventController defaults GtkGestureSingle::button is set to 0 on the multipress gesture, as several buttons are managed by that gesture. Also avoid some extra lines of code setting what nowadays are default values.
Created attachment 283193 [details] [review] spinbutton: Make the swipe gesture only work on touch devices This code was relying on the previous GtkGestureSingle::touch-only default value.
Created attachment 283194 [details] [review] scrolledwindow: Make kinetic scrolling only work on touch devices This code was relying on the previous GtkGestureSingle::touch-only default value, so restore the behavior back.
Created attachment 283195 [details] [review] window: Set GtkGestureSingle::button to 0 on multipress gesture This gesture handles events from multiple buttons, so ensure the gesture still does so after the default value change.
Created attachment 283196 [details] [review] range: Make multipress gesture accept all buttons That gesture is meant to handle clicks from several buttons, so unset the new GDK_BUTTON_PRIMARY default. Also, remove unnecessary boilerplate with the new default values.
Created attachment 283197 [details] [review] textview: Make multipress gesture accept all buttons That gesture is meant to handle clicks on multiple buttons, so unset the GDK_BUTTON_PRIMARY default. Also, remove unnecessary boilerplate with the new GtkGestureSingle/GtkEventController defaults.
Created attachment 283198 [details] [review] treeview: Make multipress gesture accept all buttons That gesture is meant to handle clicks on multiple buttons, so unset the GDK_BUTTON_PRIMARY default. Also, remove unnecessary boilerplate with the new GtkGestureSingle/GtkEventController defaults.
Review of attachment 283184 [details] [review]: ok
Review of attachment 283185 [details] [review]: ok
Review of attachment 283186 [details] [review]: ok
Review of attachment 283187 [details] [review]: ok
Review of attachment 283188 [details] [review]: ok
Review of attachment 283189 [details] [review]: ok
Review of attachment 283190 [details] [review]: could remove the now redundant setting of touch-only to FALSE, here
Review of attachment 283191 [details] [review]: ok
Review of attachment 283192 [details] [review]: ok
Review of attachment 283193 [details] [review]: ok
Review of attachment 283194 [details] [review]: ok
Review of attachment 283195 [details] [review]: ok
Review of attachment 283196 [details] [review]: ok
Review of attachment 283197 [details] [review]: ok
Review of attachment 283198 [details] [review]: ok
Attachment 283184 [details] pushed as a050b01 - eventcontroller: Default to GTK_PHASE_BUBBLE Attachment 283185 [details] pushed as dd9166c - dnd: Set drag gesture propagation phase to GTK_PHASE_NONE explicitly Attachment 283186 [details] pushed as 131123c - window: Set multipress gesture phase to be GTK_PHASE_NONE explicitly Attachment 283187 [details] pushed as 9ad4f1e - gesturesingle: Change default values of button and touch-only properties Attachment 283188 [details] pushed as 9e89640 - colorplane: Update to new GtkGestureSingle/GtkEventController defaults Attachment 283189 [details] pushed as 1c23ab5 - colorswatch: Update to new GtkGestureSingle/GtkEventController defaults Attachment 283190 [details] pushed as 8ca205a - dnd: Set explicitly GtkGestureSingle::button to 0 on the drag gesture Attachment 283191 [details] pushed as 0c2da16 - entry: Update to new GtkGestureSingle/GtkEventController defaults Attachment 283192 [details] pushed as 261bbd8 - label: Update to new GtkGestureSingle/GtkEventController defaults Attachment 283193 [details] pushed as b7412a1 - spinbutton: Make the swipe gesture only work on touch devices Attachment 283194 [details] pushed as e3a281b - scrolledwindow: Make kinetic scrolling only work on touch devices Attachment 283195 [details] pushed as c17d3ee - window: Set GtkGestureSingle::button to 0 on multipress gesture Attachment 283196 [details] pushed as af5a03c - range: Make multipress gesture accept all buttons Attachment 283197 [details] pushed as c583970 - textview: Make multipress gesture accept all buttons Attachment 283198 [details] pushed as 91afc0e - treeview: Make multipress gesture accept all buttons