Issue #11906: action search followed by down arrow key stops responding…

… to keyboard input.

Removing a GtkBindingSet entry actually removes it from all GtkTreeView.
It's not limited to the ones contained in GimpContainerTreeView (which
makes sense, since it relies on the class) so it broke various
interactions across the software.

Let's just connect to signal key-press-event instead and block further
propagation. Not sure anyway why GTK has so many ways to handle key
interactions!

I'm also moving the implementation to GimpItemTreeView because this
enhanced keyboard handling is really about the items tree dockables
only, for the time being at least.
This commit is contained in:
Jehan 2024-08-14 23:34:27 +02:00
parent 9edb11ac5f
commit 51c8709e05
7 changed files with 140 additions and 206 deletions

View File

@ -102,10 +102,10 @@ gimp_channel_tree_view_class_init (GimpChannelTreeViewClass *klass)
view_class->drop_viewables = gimp_channel_tree_view_drop_viewables;
view_class->drop_component = gimp_channel_tree_view_drop_component;
view_class->move_cursor_up_action = "channels-select-previous";
view_class->move_cursor_down_action = "channels-select-next";
view_class->move_cursor_start_action = "channels-select-top";
view_class->move_cursor_end_action = "channels-select-bottom";
iv_class->move_cursor_up_action = "channels-select-previous";
iv_class->move_cursor_down_action = "channels-select-next";
iv_class->move_cursor_start_action = "channels-select-top";
iv_class->move_cursor_end_action = "channels-select-bottom";
iv_class->set_image = gimp_channel_tree_view_set_image;

View File

@ -54,7 +54,6 @@
enum
{
EDIT_NAME,
MOVE_CURSOR,
LAST_SIGNAL
};
@ -99,9 +98,6 @@ static void gimp_container_tree_view_clear_items (GimpContainerVi
static void gimp_container_tree_view_set_view_size (GimpContainerView *view);
static void gimp_container_tree_view_real_edit_name (GimpContainerTreeView *tree_view);
static gboolean gimp_container_tree_view_real_move_cursor (GimpContainerTreeView *tree_view,
GtkMovementStep step,
gint count);
static gboolean gimp_container_tree_view_edit_focus_out (GtkWidget *widget,
GdkEvent *event,
@ -194,7 +190,6 @@ gimp_container_tree_view_class_init (GimpContainerTreeViewClass *klass)
widget_class->popup_menu = gimp_container_tree_view_popup_menu;
klass->edit_name = gimp_container_tree_view_real_edit_name;
klass->move_cursor = gimp_container_tree_view_real_move_cursor;
klass->drop_possible = gimp_container_tree_view_real_drop_possible;
klass->drop_viewables = gimp_container_tree_view_real_drop_viewables;
klass->drop_color = NULL;
@ -203,13 +198,6 @@ gimp_container_tree_view_class_init (GimpContainerTreeViewClass *klass)
klass->drop_component = NULL;
klass->drop_pixbuf = NULL;
klass->move_cursor_up_action = NULL;
klass->move_cursor_down_action = NULL;
klass->move_cursor_up_flat_action = NULL;
klass->move_cursor_down_flat_action = NULL;
klass->move_cursor_start_action = NULL;
klass->move_cursor_end_action = NULL;
tree_view_signals[EDIT_NAME] =
g_signal_new ("edit-name",
G_TYPE_FROM_CLASS (klass),
@ -217,89 +205,11 @@ gimp_container_tree_view_class_init (GimpContainerTreeViewClass *klass)
G_STRUCT_OFFSET (GimpContainerTreeViewClass, edit_name),
NULL, NULL, NULL,
G_TYPE_NONE, 0);
/**
* GimpContainerTreeView::move-cursor:
* @tree_view: the object on which the signal is emitted.
* @step: the granularity of the move, as a #GtkMovementStep.
* %GTK_MOVEMENT_DISPLAY_LINES, %GTK_MOVEMENT_PAGES and
* %GTK_MOVEMENT_BUFFER_ENDS are supported.
* @direction: the direction to move: +1 to move forwards;
* -1 to move backwards. The resulting movement is
* undefined for all other values.
*
* This overrides the #GtkTreeView::move-cursor signal to have
* different cursor behavior in various cases.
*
* Returns: %TRUE if @step is supported, %FALSE otherwise.
*/
tree_view_signals[MOVE_CURSOR] =
g_signal_new ("move-cursor",
G_TYPE_FROM_CLASS (klass),
G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION,
G_STRUCT_OFFSET (GimpContainerTreeViewClass, move_cursor),
NULL, NULL, NULL,
G_TYPE_BOOLEAN, 2,
GTK_TYPE_MOVEMENT_STEP,
G_TYPE_INT);
binding_set = gtk_binding_set_by_class (klass);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_F2, 0,
"edit-name", 0);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_Up, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_DISPLAY_LINES,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_Up, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_DISPLAY_LINES,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_Page_Up, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_PAGES,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_Page_Up, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_PAGES,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_Down, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_DISPLAY_LINES,
G_TYPE_INT, 1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_Down, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_DISPLAY_LINES,
G_TYPE_INT, 1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_Page_Down, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_PAGES,
G_TYPE_INT, 1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_Page_Down, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_PAGES,
G_TYPE_INT, 1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_Home, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_BUFFER_ENDS,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_Home, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_BUFFER_ENDS,
G_TYPE_INT, -1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_End, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_BUFFER_ENDS,
G_TYPE_INT, 1);
gtk_binding_entry_add_signal (binding_set, GDK_KEY_KP_End, 0,
"move-cursor", 2,
G_TYPE_ENUM, GTK_MOVEMENT_BUFFER_ENDS,
G_TYPE_INT, 1);
}
static void
@ -352,8 +262,6 @@ gimp_container_tree_view_constructed (GObject *object)
GimpContainerTreeView *tree_view = GIMP_CONTAINER_TREE_VIEW (object);
GimpContainerView *view = GIMP_CONTAINER_VIEW (object);
GimpContainerBox *box = GIMP_CONTAINER_BOX (object);
GtkTreeViewClass *gtk_tree_view_class;
GtkBindingSet *binding_set;
G_OBJECT_CLASS (parent_class)->constructed (object);
@ -370,32 +278,6 @@ gimp_container_tree_view_constructed (GObject *object)
"show-expanders", GIMP_CONTAINER_VIEW_GET_IFACE (view)->model_is_tree,
NULL);
gtk_tree_view_class = GTK_TREE_VIEW_GET_CLASS (tree_view->view);
binding_set = gtk_binding_set_by_class (gtk_tree_view_class);
/* Remove various of the bindings created in
* gtk_tree_view_class_init() to recreate the correct behaviour for
* us.
*/
gtk_binding_entry_remove (binding_set, GDK_KEY_Up, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_Up, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_Down, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_Down, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_Home, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_Home, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_End, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_End, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_Page_Up, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_Page_Up, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_Page_Down, 0);
gtk_binding_entry_remove (binding_set, GDK_KEY_KP_Page_Down, 0);
gtk_container_add (GTK_CONTAINER (box->scrolled_win),
GTK_WIDGET (tree_view->view));
gtk_widget_show (GTK_WIDGET (tree_view->view));
@ -1285,68 +1167,6 @@ gimp_container_tree_view_real_edit_name (GimpContainerTreeView *tree_view)
gtk_widget_error_bell (GTK_WIDGET (tree_view));
}
static gboolean
gimp_container_tree_view_real_move_cursor (GimpContainerTreeView *tree_view,
GtkMovementStep step,
gint count)
{
GimpContainerTreeViewClass *klass;
const gchar *action_name = NULL;
g_return_val_if_fail (GIMP_IS_CONTAINER_TREE_VIEW (tree_view), FALSE);
g_return_val_if_fail (step == GTK_MOVEMENT_DISPLAY_LINES ||
step == GTK_MOVEMENT_PAGES ||
step == GTK_MOVEMENT_BUFFER_ENDS, FALSE);
klass = GIMP_CONTAINER_TREE_VIEW_GET_CLASS (tree_view);
switch (step)
{
case GTK_MOVEMENT_DISPLAY_LINES:
if (count > 0)
{
if (klass->move_cursor_down_flat_action)
action_name = klass->move_cursor_down_flat_action;
else
action_name = klass->move_cursor_down_action;
}
else
{
if (klass->move_cursor_up_flat_action)
action_name = klass->move_cursor_up_flat_action;
else
action_name = klass->move_cursor_up_action;
}
break;
case GTK_MOVEMENT_PAGES:
if (count > 0)
action_name = klass->move_cursor_down_action;
else
action_name = klass->move_cursor_up_action;
break;
case GTK_MOVEMENT_BUFFER_ENDS:
if (count > 0)
action_name = klass->move_cursor_end_action;
else
action_name = klass->move_cursor_start_action;
break;
default:
break;
}
if (action_name != NULL)
{
GAction *action;
action = g_action_map_lookup_action (G_ACTION_MAP (g_application_get_default ()),
action_name);
g_return_val_if_fail (action != NULL, FALSE);
gimp_action_activate (GIMP_ACTION (action));
}
return TRUE;
}
/* callbacks */

View File

@ -61,9 +61,6 @@ struct _GimpContainerTreeViewClass
/* signals */
void (* edit_name) (GimpContainerTreeView *tree_view);
gboolean (* move_cursor) (GimpContainerTreeView *tree_view,
GtkMovementStep step,
gint count);
/* virtual functions */
@ -101,15 +98,6 @@ struct _GimpContainerTreeViewClass
GdkPixbuf *pixbuf,
GimpViewable *dest_viewable,
GtkTreeViewDropPosition drop_pos);
/* actions */
const gchar * move_cursor_up_action;
const gchar * move_cursor_down_action;
const gchar * move_cursor_up_flat_action;
const gchar * move_cursor_down_flat_action;
const gchar * move_cursor_start_action;
const gchar * move_cursor_end_action;
};

View File

@ -183,6 +183,9 @@ static void gimp_item_tree_view_style_updated (GtkWidget *widget);
static void gimp_item_tree_view_real_set_image (GimpItemTreeView *view,
GimpImage *image);
static gboolean gimp_item_tree_view_key_press_event (GtkTreeView *tree_view,
GdkEventKey *event,
GimpItemTreeView *view);
static void gimp_item_tree_view_image_flush (GimpImage *image,
gboolean invalidate_preview,
@ -360,6 +363,10 @@ static gboolean gimp_item_tree_view_search_clicked (GtkWidget
GdkEventButton *event,
GimpItemTreeView *view);
static gboolean gimp_item_tree_view_move_cursor (GimpItemTreeView *tree_view,
guint keyval,
GdkModifierType modifiers);
G_DEFINE_TYPE_WITH_CODE (GimpItemTreeView, gimp_item_tree_view,
GIMP_TYPE_CONTAINER_TREE_VIEW,
@ -435,6 +442,13 @@ gimp_item_tree_view_class_init (GimpItemTreeViewClass *klass)
klass->lock_visibility_icon_name = NULL;
klass->lock_visibility_tooltip = NULL;
klass->lock_visibility_help_id = NULL;
klass->move_cursor_up_action = NULL;
klass->move_cursor_down_action = NULL;
klass->move_cursor_up_flat_action = NULL;
klass->move_cursor_down_flat_action = NULL;
klass->move_cursor_start_action = NULL;
klass->move_cursor_end_action = NULL;
}
static void
@ -520,6 +534,10 @@ gimp_item_tree_view_constructed (GObject *object)
G_OBJECT_CLASS (parent_class)->constructed (object);
g_signal_connect (tree_view->view, "key-press-event",
G_CALLBACK (gimp_item_tree_view_key_press_event),
tree_view);
gtk_tree_view_set_headers_visible (tree_view->view, TRUE);
gtk_widget_style_get (GTK_WIDGET (item_view),
@ -1463,6 +1481,44 @@ gimp_item_tree_view_real_set_image (GimpItemTreeView *view,
gimp_item_tree_view_floating_selection_changed (view->priv->image, view);
}
static gboolean
gimp_item_tree_view_key_press_event (GtkTreeView *tree_view,
GdkEventKey *event,
GimpItemTreeView *view)
{
if (event->keyval == GDK_KEY_Up ||
event->keyval == GDK_KEY_KP_Up ||
event->keyval == GDK_KEY_Down ||
event->keyval == GDK_KEY_KP_Down ||
event->keyval == GDK_KEY_Page_Up ||
event->keyval == GDK_KEY_KP_Page_Up ||
event->keyval == GDK_KEY_Down ||
event->keyval == GDK_KEY_KP_Down ||
event->keyval == GDK_KEY_Page_Down ||
event->keyval == GDK_KEY_KP_Page_Down ||
event->keyval == GDK_KEY_Home ||
event->keyval == GDK_KEY_KP_Home ||
event->keyval == GDK_KEY_End ||
event->keyval == GDK_KEY_KP_End)
{
if (event->state & gimp_get_all_modifiers_mask ())
/* Let the event propagate, though to be fair, it would be nice
* to specify what happens with a Shift-arrow, or Ctrl-arrow,
* with a multi-item aware logic. It may also require a concept
* of "active" item among the selected one (very likely the last
* clicked item).
* TODO for a proper gimp-ux spec.
*/
return FALSE;
gimp_item_tree_view_move_cursor (view, event->keyval, event->state);
return TRUE;
}
return FALSE;
}
static void
gimp_item_tree_view_image_flush (GimpImage *image,
gboolean invalidate_preview,
@ -3754,3 +3810,64 @@ gimp_item_tree_view_search_clicked (GtkWidget *main_column_button,
return TRUE;
}
static gboolean
gimp_item_tree_view_move_cursor (GimpItemTreeView *view,
guint keyval,
GdkModifierType modifiers)
{
GimpItemTreeViewClass *klass;
const gchar *action_name = NULL;
g_return_val_if_fail (GIMP_IS_ITEM_TREE_VIEW (view), FALSE);
klass = GIMP_ITEM_TREE_VIEW_GET_CLASS (view);
switch (keyval)
{
case GDK_KEY_Down:
case GDK_KEY_KP_Down:
if (klass->move_cursor_down_flat_action)
action_name = klass->move_cursor_down_flat_action;
else
action_name = klass->move_cursor_down_action;
break;
case GDK_KEY_Up:
case GDK_KEY_KP_Up:
if (klass->move_cursor_up_flat_action)
action_name = klass->move_cursor_up_flat_action;
else
action_name = klass->move_cursor_up_action;
break;
case GDK_KEY_Page_Down:
case GDK_KEY_KP_Page_Down:
action_name = klass->move_cursor_down_action;
break;
case GDK_KEY_Page_Up:
case GDK_KEY_KP_Page_Up:
action_name = klass->move_cursor_up_action;
break;
case GDK_KEY_End:
case GDK_KEY_KP_End:
action_name = klass->move_cursor_end_action;
break;
case GDK_KEY_Home:
case GDK_KEY_KP_Home:
action_name = klass->move_cursor_start_action;
break;
default:
break;
}
if (action_name != NULL)
{
GAction *action;
action = g_action_map_lookup_action (G_ACTION_MAP (g_application_get_default ()),
action_name);
g_return_val_if_fail (action != NULL, FALSE);
gimp_action_activate (GIMP_ACTION (action));
}
return TRUE;
}

View File

@ -117,6 +117,15 @@ struct _GimpItemTreeViewClass
const gchar *lock_visibility_icon_name;
const gchar *lock_visibility_tooltip;
const gchar *lock_visibility_help_id;
/* actions */
const gchar *move_cursor_up_action;
const gchar *move_cursor_down_action;
const gchar *move_cursor_up_flat_action;
const gchar *move_cursor_down_flat_action;
const gchar *move_cursor_start_action;
const gchar *move_cursor_end_action;
};

View File

@ -208,12 +208,12 @@ gimp_layer_tree_view_class_init (GimpLayerTreeViewClass *klass)
tree_view_class->drop_component = gimp_layer_tree_view_drop_component;
tree_view_class->drop_pixbuf = gimp_layer_tree_view_drop_pixbuf;
tree_view_class->move_cursor_up_action = "layers-select-previous";
tree_view_class->move_cursor_down_action = "layers-select-next";
tree_view_class->move_cursor_up_flat_action = "layers-select-flattened-previous";
tree_view_class->move_cursor_down_flat_action = "layers-select-flattened-next";
tree_view_class->move_cursor_start_action = "layers-select-top";
tree_view_class->move_cursor_end_action = "layers-select-bottom";
item_view_class->move_cursor_up_action = "layers-select-previous";
item_view_class->move_cursor_down_action = "layers-select-next";
item_view_class->move_cursor_up_flat_action = "layers-select-flattened-previous";
item_view_class->move_cursor_down_flat_action = "layers-select-flattened-next";
item_view_class->move_cursor_start_action = "layers-select-top";
item_view_class->move_cursor_end_action = "layers-select-bottom";
item_view_class->item_type = GIMP_TYPE_LAYER;
item_view_class->signal_name = "selected-layers-changed";

View File

@ -87,10 +87,10 @@ gimp_path_tree_view_class_init (GimpPathTreeViewClass *klass)
view_class->drop_svg = gimp_path_tree_view_drop_svg;
view_class->move_cursor_up_action = "paths-select-previous";
view_class->move_cursor_down_action = "paths-select-next";
view_class->move_cursor_start_action = "paths-select-top";
view_class->move_cursor_end_action = "paths-select-bottom";
iv_class->move_cursor_up_action = "paths-select-previous";
iv_class->move_cursor_down_action = "paths-select-next";
iv_class->move_cursor_start_action = "paths-select-top";
iv_class->move_cursor_end_action = "paths-select-bottom";
iv_class->item_type = GIMP_TYPE_PATH;
iv_class->signal_name = "selected-paths-changed";