Fix keyboard focus issues with a demo page for keyboard focus

docs/index.rst

@@ -716,6 +716,9 @@ Many widgets and layouts share common properties. These common properties are de
     A string value indicating the font for the widget. Bindable.
 ``border_color``
     A string value indicating the border color for the widget. Used for debugging. Bindable.
+``on_focus_changed``
+    The name of a handler method called when the widget gains or loses the keyboard focus. The widget and a boolean
+    saying whether it is now focused are passed to the method.
 
 Resources
 ^^^^^^^^^

nion/ui/Declarative.py

@@ -84,7 +84,7 @@ class DeclarativeUI:
     # TODO: thumbnails
     # TODO: display panels
     # TODO: periodic
-    # TODO: focus handler
+    # ----: focus handler
     # ----: bindings
     # TODO: commands
     # TODO: standard dialog boxes, open, save, print, confirm
@@ -119,7 +119,8 @@ def __process_common_properties(self, d: typing.MutableMapping[str, typing.Any],
             "background_color",
             "border_color",
             "widget_id",
-            "style"
+            "style",
+            "on_focus_changed",
         )
         for k in common_properties:
             if k in kwargs and kwargs[k] is not None:
@@ -1323,6 +1324,9 @@ def connect_attributes(widget: UserInterface.Widget, d: UIDescription, handler:
     connect_reference_value(widget, d, handler, "border_color", finishes, value_type=str)
     connect_reference_value(widget, d, handler, "color", finishes, value_type=str)
     connect_reference_value(widget, d, handler, "font", finishes, value_type=str)
+    # every widget reports when it gains or loses the keyboard focus, so the callback is connected here rather
+    # than by each widget in turn.
+    connect_event(widget, widget, d, handler, "on_focus_changed", ["focused"])
     widget.widget_id = d.get("widget_id", widget.widget_id)
 
 

nion/ui/Widgets.py

@@ -236,6 +236,67 @@ def apply_sizing_properties(canvas_item: CanvasItem.AbstractCanvasItem, properti
         canvas_item.update_sizing(canvas_item_sizing.with_unconstrained_height().with_preferred_height(min(max_height, preferred_height)).with_maximum_height(max_height))
 
 
+class ControlCanvasItem(CanvasItem.CanvasItemComposition):
+    """The canvas item drawing a control made of several cells: a push button of an icon and a text, say, or a
+    combo box of a text and a triangle.
+
+    The cells are drawn separately but the control takes the focus and is activated as a single unit, so the focus
+    and the keys are handled here rather than by any one of the cells.
+
+    A control like this has nothing selected to show the focus on, so it shows it as a stronger border around
+    itself, drawn in the same color as the focus ring of the controls which draw one. The border it shows when it
+    is not focused is its base border, which is what the widget drawing the control sets.
+    """
+
+    def __init__(self) -> None:
+        super().__init__()
+        self.focusable = True
+        # a click presses the control rather than settling into it, so it is reached by the keyboard only by
+        # walking the focus to it; clicking it leaves the focus wherever the user was working.
+        self.takes_focus_on_click = False
+        self.on_clicked: typing.Optional[typing.Callable[[], None]] = None
+        self.__base_border_color: typing.Optional[str] = None
+        self.__base_border_width: typing.Optional[float] = None
+
+    def close(self) -> None:
+        self.on_clicked = None
+        super().close()
+
+    @property
+    def base_border_color(self) -> typing.Optional[str]:
+        return self.__base_border_color
+
+    @base_border_color.setter
+    def base_border_color(self, base_border_color: typing.Optional[str]) -> None:
+        self.__base_border_color = base_border_color
+        self.__update_border()
+
+    @property
+    def base_border_width(self) -> typing.Optional[float]:
+        return self.__base_border_width
+
+    @base_border_width.setter
+    def base_border_width(self, base_border_width: typing.Optional[float]) -> None:
+        self.__base_border_width = base_border_width
+        self.__update_border()
+
+    def _set_focused(self, focused: bool) -> None:
+        super()._set_focused(focused)
+        self.__update_border()
+
+    def __update_border(self) -> None:
+        self.border_color = CanvasItem.FOCUS_RING_COLOR if self.focused else self.__base_border_color
+        self.border_width = CanvasItem.FOCUS_RING_WIDTH if self.focused else self.__base_border_width
+
+    def key_pressed(self, key: UserInterface.Key) -> bool:
+        # the space bar and return activate the control, the way clicking it does.
+        if self.enabled and (key.text == " " or key.is_enter_or_return):
+            if callable(self.on_clicked):
+                self.on_clicked()
+            return True
+        return super().key_pressed(key)
+
+
 class BasicPushButtonWidgetCanvasItemController(PushButtonWidgetCanvasItemController):
 
     # matches a typical native push button's default minimum width so that short-text buttons
@@ -256,14 +317,15 @@ def __init__(self, ui: UserInterface.UserInterface, properties: typing.Optional[
         # margins instead, applied once around the combo rather than doubled up per item.
         self.__text_button_canvas_item = CanvasItem.TextButtonCanvasItem(padding=Geometry.IntSize(height=4, width=0), group_controller=self.__group_controller)
         self.__icon_button_canvas_item = CanvasItem.BitmapButtonCanvasItem(padding=Geometry.IntSize(height=4, width=0), group_controller=self.__group_controller)
-        self.__stack = CanvasItem.CanvasItemComposition()
+        self.__stack = ControlCanvasItem()
         self.__stack.layout = CanvasItem.CanvasItemRowLayout(margins=Geometry.Margins(top=0, left=8, bottom=0, right=8))
         # the "base" background is the button's normal, non-hovered appearance; it is what
         # set_background_color changes, and what the hover/press tint is computed relative to.
         self.__base_background_color: typing.Optional[typing.Union[str, DrawingContext.LinearGradient]] = "white"
         self.__stack.background_color = self.__base_background_color
-        self.__stack.border_color = "#c0c0c0"
-        self.__stack.border_width = 0.5
+        # the base border is the button's normal, unfocused appearance; the focus draws over it.
+        self.__stack.base_border_color = "#c0c0c0"
+        self.__stack.base_border_width = 0.5
         self.__stack.corner_radius = 3
         # icon (if any) is shown to the left of the text (if any); both can be visible at once.
         self.__stack.add_canvas_item(self.__icon_button_canvas_item)
@@ -292,6 +354,8 @@ def handle_style_changed(hover: bool, pressed: bool) -> None:
 
         self.__group_controller.on_clicked = handle_clicked
         self.__group_controller.on_style_changed = handle_style_changed
+        # the button can also be pressed by the keyboard, which the cells within it never see.
+        self.__stack.on_clicked = handle_clicked
 
     @property
     def widget_source(self) -> WidgetSource:
@@ -351,6 +415,8 @@ def set_icon(self, bitmap: typing.Optional[Bitmap.BitmapOrArray]) -> None:
     def set_enabled(self, enabled: bool) -> None:
         self.__text_button_canvas_item.enabled = enabled
         self.__icon_button_canvas_item.enabled = enabled
+        # the button as a whole is what the keyboard presses, so it has to know whether it can be pressed.
+        self.__stack.enabled = enabled
 
     def set_tool_tip(self, tool_tip: typing.Optional[str]) -> None:
         self.__text_button_canvas_item.tool_tip = tool_tip
@@ -361,7 +427,7 @@ def set_background_color(self, background_color: typing.Optional[typing.Union[st
         self.__stack.background_color = background_color
 
     def set_border_color(self, border_color: typing.Optional[str]) -> None:
-        self.__stack.border_color = border_color
+        self.__stack.base_border_color = border_color
 
 
 class TabWidgetCanvasItemController(BaseWidgetCanvasItemController):
@@ -749,7 +815,7 @@ def key_pressed_event(self, key_event: GridFlowCanvasItem.GridFlowCanvasItemKeyP
             if key_event.key.is_escape:
                 return list_view_widget._handle_escape_pressed()
             if key_event.key.is_enter_or_return:
-                return list_view_widget._handle_item_selected(key_event.item)
+                return list_view_widget._handle_return_pressed(key_event.item)
         return False
 
     def item_tool_tip(self, item: typing.Any) -> typing.Optional[str]:
@@ -809,6 +875,10 @@ def __init__(self, ui: UserInterface.UserInterface, list_model: ListModel.ListMo
         if v_scroll_enabled:
             scroll_group_canvas_item.add_canvas_item(CanvasItem.ScrollBarCanvasItem(scroll_area_canvas_item))
         canvas_widget = ui.create_canvas_widget(properties=properties)
+        # the list canvas item takes the keyboard focus, but it can only be reached through the widget drawing it,
+        # so that widget has to be able to take the focus too. without this the list is skipped by the tab order
+        # and can be focused only by clicking it.
+        canvas_widget.focusable = True
         canvas_widget.canvas_item.add_canvas_item(scroll_group_canvas_item)
         column_widget.add(canvas_widget)
         self.__canvas_widget = canvas_widget
@@ -835,9 +905,19 @@ def selection_changed() -> None:
 
         self.__selection_changed_event_listener = self.__selection.changed_event.listen(selection_changed)
 
+        # the list canvas item is what actually takes the keyboard focus within the canvas widget, so it is what
+        # reports the focus changing. the composite behavior wrapping the column has no focus of its own to report.
+        def focus_changed() -> None:
+            if callable(self.on_focus_changed):
+                self.on_focus_changed(self.__list_canvas_item.focused)
+
+        self.__focus_changed_event_listener = self.__list_canvas_item.focus_changed_event.listen(focus_changed)
+
         self.current_index = self.__selection.current_index
 
     def close(self) -> None:
+        self.__focus_changed_event_listener.close()
+        self.__focus_changed_event_listener = typing.cast(typing.Any, None)
         self.__selection_changed_event_listener.close()
         self.__selection_changed_event_listener = typing.cast(typing.Any, None)
         self.__current_index_binding_helper.close()
@@ -865,9 +945,14 @@ def __index_for_item(self, item: typing.Any) -> typing.Optional[int]:
     def _handle_item_selected(self, item: typing.Any) -> bool:
         # the item was chosen, by double click or by pressing return.
         index = self.__index_for_item(item)
-        handled = False
         if index is not None and callable(self.on_item_selected):
-            handled = bool(self.on_item_selected(index))
+            return bool(self.on_item_selected(index))
+        return False
+
+    def _handle_return_pressed(self, item: typing.Any) -> bool:
+        # return chooses the item under the selection and is reported as the return key too. a double click chooses
+        # the item the same way, but is not a key press, so it is not reported as one.
+        handled = self._handle_item_selected(item)
         if callable(self.on_return_pressed):
             handled = bool(self.on_return_pressed()) or handled
         return handled
@@ -916,13 +1001,23 @@ def unbind_current_index(self) -> None:
     def _list_canvas_item(self) -> ListCanvasItem.ListCanvasItem2:
         return self.__list_canvas_item
 
+    @property
+    def _canvas_widget(self) -> UserInterface.CanvasWidget:
+        return self.__canvas_widget
+
     @property
     def focused(self) -> bool:
-        return self.__canvas_widget.focused and self.__list_canvas_item.focused
+        # the list canvas item is what holds the focus; it is cleared when the widget drawing it loses the focus.
+        return self.__list_canvas_item.focused
 
     @focused.setter
     def focused(self, focused: bool) -> None:
-        self.__list_canvas_item.request_focus()
+        if focused:
+            self.__list_canvas_item.request_focus()
+        else:
+            # the widget drawing the list holds the keyboard focus on the list's behalf, so giving up the focus
+            # means giving up both: dropping the widget focus takes the focus off the list canvas item with it.
+            self.__canvas_widget.focused = False
 
 
 class StringListViewWidget(ListViewWidget):