Clean up the dashboard slot assignment popup

src/hatty/ui/controls/light_screen.py

@@ -31,14 +31,14 @@
 from textual.containers import Container, Horizontal, VerticalScroll
 from textual.screen import ModalScreen
 from textual.timer import Timer
-from textual.widget import Widget
 from textual.widgets import Button, Footer, Input, Label, OptionList, Static, TabbedContent, TabPane, Tabs
 from textual.widgets.option_list import Option
 from textual_colorpicker import ColorPicker
 
 from hatty.ui.controls.kelvin_slider import KelvinSlider
 from hatty.ui.controls.percentage_slider import PercentageSlider
 from hatty.ui.entity_table import get_display_name
+from hatty.ui.focus_nav import enclosing_row, focus_within_row, nav_focus
 from hatty.ui.popup_base import PopupScreen
 
 if TYPE_CHECKING:
@@ -426,30 +426,6 @@ def on_button_pressed(self, event: Button.Pressed) -> None:
             self.action_open_color_picker()
             event.stop()
 
-    def _enclosing_row(self, widget: Widget | None) -> Widget | None:
-        """The `#color_swatches`/`#white_presets` Horizontal row containing `widget`, if any."""
-        if widget is None:
-            return None
-        for node in widget.ancestors_with_self:
-            if isinstance(node, Widget) and node.id in _BUTTON_ROW_IDS:
-                return node
-        return None
-
-    def _focus_within_row(self, row: Widget, focused: Widget, step: int) -> None:
-        buttons = list(row.query(Button))
-        if not buttons:
-            return
-        index = next((i for i, button in enumerate(buttons) if button is focused), 0)
-        buttons[(index + step) % len(buttons)].focus()
-
-    def _focus_out_of_row(self, row: Widget, step: int) -> None:
-        """Move focus in `step`'s direction, skipping the whole row as one unit."""
-        step_focus = self.focus_next if step > 0 else self.focus_previous
-        for _ in range(len(self.focus_chain)):
-            landed = step_focus()
-            if landed is None or row not in landed.ancestors_with_self:
-                return
-
     def on_key(self, event: events.Key) -> None:
         # Left/right walk the focus chain unless a slider/input owns them — or the tab
         # bar, where they natively switch panes (issue #88). Up/down are handled as
@@ -459,9 +435,9 @@ def on_key(self, event: events.Key) -> None:
         if isinstance(focused, (Input, PercentageSlider, KelvinSlider, Tabs)):
             return
         if event.key in ("left", "right"):
-            row = self._enclosing_row(focused)
+            row = enclosing_row(focused, _BUTTON_ROW_IDS)
             if row is not None and focused is not None:
-                self._focus_within_row(row, focused, 1 if event.key == "right" else -1)
+                focus_within_row(row, focused, 1 if event.key == "right" else -1)
             elif event.key == "left":
                 self.focus_previous()
             else:
@@ -499,13 +475,7 @@ def action_show_help(self) -> None:
     def action_nav_focus(self, direction: int) -> None:
         # A focused slider (or any single widget) just steps one at a time; a row
         # (swatches/presets) is skipped as a whole block (issue #286).
-        row = self._enclosing_row(self.focused)
-        if row is not None:
-            self._focus_out_of_row(row, direction)
-        elif direction > 0:
-            self.focus_next()
-        else:
-            self.focus_previous()
+        nav_focus(self, _BUTTON_ROW_IDS, direction)
 
     def action_toggle_power(self) -> None:
         entity = self.app.find_entity(self._entity_id) or self._entity

src/hatty/ui/controls/media_player_screen.py

@@ -29,13 +29,13 @@
 from textual.containers import Horizontal, VerticalScroll
 from textual.screen import ModalScreen
 from textual.timer import Timer
-from textual.widget import Widget
 from textual.widgets import Button, Footer, Label, OptionList, Select, Static
 
 from hatty.const import media_supports
 from hatty.ui.controls.light_screen import DEBOUNCE_SECONDS
 from hatty.ui.controls.percentage_slider import PercentageSlider
 from hatty.ui.entity_table import get_display_name
+from hatty.ui.focus_nav import enclosing_row, focus_within_row, nav_focus
 
 if TYPE_CHECKING:
     from hatty.main import HACLI
@@ -305,9 +305,9 @@ def on_key(self, event: events.Key) -> None:
         if isinstance(focused, (PercentageSlider, OptionList)):
             return
         if event.key in ("left", "right"):
-            row = self._enclosing_row(focused)
+            row = enclosing_row(focused, _BUTTON_ROW_IDS)
             if row is not None and focused is not None:
-                self._focus_within_row(row, focused, 1 if event.key == "right" else -1)
+                focus_within_row(row, focused, 1 if event.key == "right" else -1)
             elif event.key == "left":
                 self.focus_previous()
             else:
@@ -338,38 +338,7 @@ def action_nav_focus(self, direction: int) -> None:
         # A focused slider (or any other single widget) just steps one at a time; a
         # button row (transport/toggle) is skipped as a whole block (mirrors
         # light_screen.py's #286 pattern).
-        row = self._enclosing_row(self.focused)
-        if row is not None:
-            self._focus_out_of_row(row, direction)
-        elif direction > 0:
-            self.focus_next()
-        else:
-            self.focus_previous()
-
-    def _enclosing_row(self, widget: Widget | None) -> Widget | None:
-        """The `#transport_buttons`/`#toggle_buttons` Horizontal row containing `widget`, if any."""
-        if widget is None:
-            return None
-        for node in widget.ancestors_with_self:
-            if isinstance(node, Widget) and node.id in _BUTTON_ROW_IDS:
-                return node
-        return None
-
-    def _focus_within_row(self, row: Widget, focused: Widget, step: int) -> None:
-        """Cycle focus among `row`'s buttons, wrapping at the ends."""
-        buttons = list(row.query(Button))
-        if not buttons:
-            return
-        index = next((i for i, button in enumerate(buttons) if button is focused), 0)
-        buttons[(index + step) % len(buttons)].focus()
-
-    def _focus_out_of_row(self, row: Widget, step: int) -> None:
-        """Move focus in `step`'s direction, skipping the whole row as one unit."""
-        step_focus = self.focus_next if step > 0 else self.focus_previous
-        for _ in range(len(self.focus_chain)):
-            landed = step_focus()
-            if landed is None or row not in landed.ancestors_with_self:
-                return
+        nav_focus(self, _BUTTON_ROW_IDS, direction)
 
     def action_toggle_play_pause(self) -> None:
         if not self.supports_play_pause:

src/hatty/ui/dashboard/slot_popup.py

@@ -29,6 +29,14 @@
 in a fixed pane to the right of the main column, shown only when the terminal
 is wide enough to fit both side by side (`preview_fits`, issue #11); on
 narrow terminals it's hidden outright rather than shuffled by step or type.
+
+`up`/`down` are a priority binding (issue #36) that always steps focus one
+field at a time, mirroring `light_screen.py`/`media_player_screen.py` — the
+entity table, the selected-entities list, and an open type dropdown keep
+their own up/down row cursor instead (released via `check_action`).
+`left`/`right` cycle within `#type_step_buttons` when one of its buttons is
+focused, else step focus like up/down; a focused `Input` or open dropdown
+keeps its own native left/right.
 """
 
 from typing import TYPE_CHECKING, cast
@@ -38,12 +46,13 @@
 from textual.containers import Container, Horizontal, Vertical
 from textual.events import Key
 from textual.timer import Timer
-from textual.widgets import Button, Checkbox, DataTable, Footer, Input, Label, ListItem, ListView, Select
+from textual.widgets import Button, Checkbox, DataTable, Footer, Input, Label, ListItem, ListView, OptionList, Select
 
 from hatty.const import LAST_CHANGED_WIDGET_TYPES, WIDGET_TYPES
 from hatty.ui.dashboard.widget_match import compatible_widget_types, entity_matches_widget_type
 from hatty.ui.dashboard.widgets.base import build_slot_content
 from hatty.ui.entity_table import EntitiesTable, entity_matches, get_display_name
+from hatty.ui.focus_nav import enclosing_row, focus_within_row, nav_focus
 from hatty.ui.popup_base import PopupScreen
 from hatty.ui.search_input import SearchInput
 
@@ -73,6 +82,11 @@ class DashboardSlotPopup(PopupScreen):
 
     AUTO_FOCUS = "#widget_type_select"
 
+    # The only row where left/right should cycle within it (wrapping) and up/down
+    # should jump out as a block, rather than stepping through each button (issue #36,
+    # mirrors light_screen.py/media_player_screen.py's _BUTTON_ROW_IDS convention).
+    BUTTON_ROW_IDS = ("type_step_buttons",)
+
     BINDINGS = [
         ("escape", "cancel", "Cancel"),
         Binding("q", "cancel", "Cancel", show=False),
@@ -84,14 +98,23 @@ class DashboardSlotPopup(PopupScreen):
         Binding("shift+up", "reorder_selected(-1)", "Move Up", show=False),
         Binding("shift+down", "reorder_selected(1)", "Move Down", show=False),
         Binding("delete", "remove_selected", "Remove", show=False),
+        # Priority so up/down always move focus instead of being swallowed by the
+        # entity table's/selected-list's own cursor; check_action releases it while
+        # those (or an open type dropdown) are focused so their cursor keeps working.
+        Binding("up", "nav_focus(-1)", "Focus Up", show=False, priority=True),
+        Binding("down", "nav_focus(1)", "Focus Down", show=False, priority=True),
     ]
 
     DEFAULT_CSS = """
     #dashboard_slot_container {
-        width: auto;
+        /* Width is set explicitly in Python (_apply_preview_visibility, issue #36):
+           `auto` doesn't work here — the Footer() composed inside this container is
+           full-width, so `auto` resolves to the whole screen instead of the content's
+           actual width, leaving the dialog stuck against the left edge uncentred. */
         max-width: 100%;
     }
     #slot_body {
+        width: auto;
         height: auto;
     }
     #slot_main {
@@ -364,7 +387,13 @@ def on_data_table_row_highlighted(self, event: DataTable.RowHighlighted) -> None
         self._preview_timer = self.set_timer(0.3, lambda: self._rebuild_preview(entity_id))
 
     def _apply_preview_visibility(self) -> None:
-        self.query_one("#widget_preview").display = preview_fits(self.app.size.width)
+        # Ties the dialog's width to the same show/hide decision (issue #36): with
+        # #dashboard_slot_container's width no longer `auto` (see DEFAULT_CSS), this is
+        # what centres it instead of stretching it to the terminal's full width.
+        show_preview = preview_fits(self.app.size.width)
+        self.query_one("#widget_preview").display = show_preview
+        width = MAIN_WIDTH + PREVIEW_GAP + PREVIEW_WIDTH + POPUP_CHROME if show_preview else MAIN_WIDTH + POPUP_CHROME
+        self.query_one("#dashboard_slot_container").styles.width = width
 
     def on_resize(self, event) -> None:
         # Terminal resized while the popup is open (issue #11) — re-decide
@@ -505,47 +534,42 @@ def on_select_changed(self, event: Select.Changed) -> None:
                 self._update_mode_visibility()
         self._rebuild_preview()
 
+    def check_action(self, action: str, parameters: tuple) -> bool | None:
+        if action == "nav_focus":
+            # Let the entity table, the selected-entities list, and an open type
+            # dropdown's overlay keep their own up/down row cursor.
+            return not isinstance(self.focused, (DataTable, ListView, OptionList))
+        return True
+
+    def action_nav_focus(self, direction: int) -> None:
+        nav_focus(self, self.BUTTON_ROW_IDS, direction)
+
     def on_key(self, event: Key) -> None:
-        if self._step == "type":
-            self._handle_type_step_key(event)
-        elif self._step == "entity" and self._is_multi_add():
-            focused = self.focused
+        focused = self.focused
+        # Multi-add's search <-> Done shortcut (issue #254): the entity table below
+        # keeps its own row cursor, so down can't reach Done from the search box.
+        if self._step == "entity" and self._is_multi_add():
             if event.key == "right" and focused is self.query_one("#entity_search_input"):
                 self.set_focus(self.query_one("#btn_panel_done"))
                 event.prevent_default()
+                return
             elif event.key == "left" and focused is self.query_one("#btn_panel_done"):
                 self.set_focus(self.query_one("#entity_search_input"))
                 event.prevent_default()
-
-    def _handle_type_step_key(self, event: Key) -> None:
-        select = self.query_one("#widget_type_select")
-        next_button = self.query_one("#btn_next_step")
-        focused = self.focused
-        # No third stop once "Pick Entity First" is gone: fill mode never
-        # composes it, and entity-first's revisited type step hides it.
-        entity_first_button = None if self._fill_mode or self._entity_first else self.query_one("#btn_entity_first")
-        if entity_first_button is None:
-            if event.key == "right" and focused is select:
-                self.set_focus(next_button)
-                event.prevent_default()
-            elif event.key == "left" and focused is next_button:
-                self.set_focus(select)
-                event.prevent_default()
+                return
+        # A focused Input or an open type dropdown's overlay keep their own native
+        # left/right (cursor movement, option highlight); everything else either
+        # cycles within its enclosing button row or steps focus by one field.
+        if event.key not in ("left", "right") or isinstance(focused, (Input, OptionList)):
             return
-        if event.key == "right":
-            if focused is select:
-                self.set_focus(next_button)
-                event.prevent_default()
-            elif focused is next_button:
-                self.set_focus(entity_first_button)
-                event.prevent_default()
+        row = enclosing_row(focused, self.BUTTON_ROW_IDS)
+        if row is not None and focused is not None:
+            focus_within_row(row, focused, 1 if event.key == "right" else -1)
         elif event.key == "left":
-            if focused is entity_first_button:
-                self.set_focus(next_button)
-                event.prevent_default()
-            elif focused is next_button:
-                self.set_focus(select)
-                event.prevent_default()
+            self.focus_previous()
+        else:
+            self.focus_next()
+        event.stop()
 
     def on_button_pressed(self, event: Button.Pressed) -> None:
         if event.button.id == "btn_next_step":

src/hatty/ui/focus_nav.py

@@ -0,0 +1,57 @@
+# hatty — MIT License. See LICENSE file for details.
+"""Shared row-aware focus navigation for modal screens with button rows (issue #36).
+
+The convention (established by `light_screen.py`/`media_player_screen.py`, issues #88/#286):
+left/right cycle within a `Horizontal` button row (wrapping) but hand off to a focused
+widget's own native handling (a slider, an `Input`, an expanded `Select`'s overlay) when one
+of those owns focus instead; up/down are a priority `Binding` that always steps focus one
+field at a time, treating a button row as a single stop rather than one button at a time —
+`check_action` is how each screen releases `nav_focus` while a widget with its own up/down
+cursor (an `OptionList`, a `DataTable`, a `ListView`) is focused.
+"""
+
+from textual.screen import Screen
+from textual.widget import Widget
+from textual.widgets import Button
+
+
+def enclosing_row(widget: Widget | None, row_ids: tuple[str, ...]) -> Widget | None:
+    """The `row_ids`-tagged `Horizontal` containing `widget`, if any."""
+    if widget is None:
+        return None
+    for node in widget.ancestors_with_self:
+        if isinstance(node, Widget) and node.id in row_ids:
+            return node
+    return None
+
+
+def focus_within_row(row: Widget, focused: Widget, step: int) -> None:
+    """Cycle focus among `row`'s buttons, wrapping at the ends. Skips buttons
+    hidden via `.display` — light_screen/media_player_screen's rows never hide
+    a composed button, but DashboardSlotPopup's `#btn_entity_first` can be."""
+    buttons = [button for button in row.query(Button) if button.display]
+    if not buttons:
+        return
+    index = next((i for i, button in enumerate(buttons) if button is focused), 0)
+    buttons[(index + step) % len(buttons)].focus()
+
+
+def focus_out_of_row(screen: Screen, row: Widget, step: int) -> None:
+    """Move focus in `step`'s direction, skipping the whole row as one unit."""
+    step_focus = screen.focus_next if step > 0 else screen.focus_previous
+    for _ in range(len(screen.focus_chain)):
+        landed = step_focus()
+        if landed is None or row not in landed.ancestors_with_self:
+            return
+
+
+def nav_focus(screen: Screen, row_ids: tuple[str, ...], direction: int) -> None:
+    """The shared `action_nav_focus` body: a focused row is skipped as a whole block;
+    anything else just steps one field at a time."""
+    row = enclosing_row(screen.focused, row_ids)
+    if row is not None:
+        focus_out_of_row(screen, row, direction)
+    elif direction > 0:
+        screen.focus_next()
+    else:
+        screen.focus_previous()