Enhance filter modal: prevent stacking and improve tab navigation

src/hosts/tui/app.py

@@ -517,6 +517,8 @@ class HostsManagerApp(App):
 
     def action_show_filters(self) -> None:
         """Open the advanced filter controls and apply their returned options."""
+        if isinstance(self.screen, FilterModal):
+            return
 
         def handle_filter_result(filter_options: FilterOptions | None) -> None:
             if filter_options is None:

src/hosts/tui/filter_modal.py

@@ -6,17 +6,15 @@ filtering options including status, type, resolution status, and search filterin
 """
 
 from textual.app import ComposeResult
-from textual.containers import Grid, Horizontal, Container
+from textual.containers import Horizontal, Vertical, VerticalScroll
 from textual.widgets import (
     Static,
     Button,
     Checkbox,
     Input,
     Select,
-    Label,
     RadioSet,
     RadioButton,
-    Collapsible,
 )
 from textual.screen import ModalScreen
 from textual import on
@@ -24,131 +22,19 @@ from textual.binding import Binding
 from typing import Optional, Dict, List
 
 from ..core.filters import FilterOptions, EntryFilter
+from .styles import FILTER_MODAL_CSS
 
 
 class FilterModal(ModalScreen[Optional[FilterOptions]]):
     """Advanced filtering configuration modal."""
 
-    BINDINGS = [Binding("escape", "cancel", "Cancel")]
+    BINDINGS = [
+        Binding("escape", "cancel", "Cancel"),
+        Binding("tab", "focus_next", show=False),
+        Binding("shift+tab", "focus_previous", show=False),
+    ]
 
-    DEFAULT_CSS = """
+    CSS = FILTER_MODAL_CSS
-    FilterModal {
-        align: center middle;
-    }
-
-    #filter-dialog {
-        grid-size: 1;
-        grid-gutter: 1 2;
-        grid-rows: auto 1fr auto;
-        padding: 0 1;
-        width: 80;
-        height: auto;
-        border: thick $background 80%;
-        background: $surface;
-        max-height: 90%;
-    }
-
-    #filter-header {
-        dock: top;
-        width: 1fr;
-        height: 3;
-        content-align: center middle;
-        text-style: bold;
-        background: $primary;
-        color: $text;
-    }
-
-    #filter-content {
-        layout: vertical;
-        overflow-y: auto;
-        height: auto;
-        max-height: 70vh;
-        padding: 1;
-    }
-
-    #filter-actions {
-        dock: bottom;
-        layout: horizontal;
-        width: 1fr;
-        height: 3;
-        align: center middle;
-        padding: 0 1;
-        background: $panel;
-    }
-
-    .filter-section {
-        margin: 1 0;
-        padding: 1;
-        border: round $primary 20%;
-        background: $panel;
-    }
-
-    .filter-section-title {
-        text-style: bold;
-        color: $primary;
-        margin-bottom: 1;
-    }
-
-    .filter-checkboxes {
-        layout: vertical;
-        margin: 0 2;
-    }
-
-    .filter-radios {
-        layout: vertical;
-        margin: 0 2;
-    }
-
-    .filter-input-row {
-        layout: horizontal;
-        margin: 0 2;
-        height: 3;
-        align: left middle;
-    }
-
-    .filter-input-label {
-        width: 20;
-        content-align: left middle;
-        margin-right: 1;
-    }
-
-    .filter-input {
-        width: 30;
-    }
-
-    .preset-row {
-        layout: horizontal;
-        margin: 1 2;
-        height: 3;
-        align: left middle;
-    }
-
-    .preset-select {
-        width: 30;
-        margin-right: 2;
-    }
-
-    Button {
-        margin: 0 1;
-        min-width: 12;
-    }
-
-    Checkbox {
-        margin: 0 1;
-    }
-
-    RadioButton {
-        margin: 0 1;
-    }
-
-    .count-display {
-        text-style: italic;
-        color: $text-muted;
-        content-align: center middle;
-        height: 1;
-        margin: 1 0;
-    }
-    """
 
     def __init__(
         self,
@@ -171,133 +57,140 @@ class FilterModal(ModalScreen[Optional[FilterOptions]]):
 
     def compose(self) -> ComposeResult:
         """Compose the filter modal interface."""
-        with Grid(id="filter-dialog"):
+        with VerticalScroll(classes="filter-container"):
-            yield Static("Advanced Filtering", id="filter-header")
+            yield Static("Advanced Filtering", classes="filter-title")
+            yield Static("", id="count-display", classes="filter-count")
 
-            with Container(id="filter-content"):
+            with Vertical(classes="default-flex-section") as presets:
-                # Filter presets section
+                presets.border_title = "Presets"
-                with Collapsible(title="Filter Presets", collapsed=False):
+                with Horizontal(classes="filter-preset-row"):
-                    with Container(classes="filter-section"):
-                        with Horizontal(classes="preset-row"):
-                            yield Label("Preset:", classes="filter-input-label")
                     yield self._create_preset_select()
-                            yield Button("Load", id="load-preset", variant="primary")
+                    yield Button(
-                            yield Button("Save", id="save-preset")
+                        "Load", id="load-preset", variant="primary", compact=True
-                            yield Button("Delete", id="delete-preset", variant="error")
+                    )
+                    yield Button("Save", id="save-preset", compact=True)
+                    yield Button(
+                        "Delete",
+                        id="delete-preset",
+                        variant="error",
+                        compact=True,
+                    )
 
-                # Status filtering section
+            with Vertical(classes="default-flex-section") as status:
-                with Collapsible(title="Status Filtering", collapsed=False):
+                status.border_title = "Entry status"
-                    with Container(classes="filter-section"):
+                with RadioSet(id="status-filter-type", classes="default-radio-set"):
-                        yield Static("Status Filtering", classes="filter-section-title")
+                    yield RadioButton("Show all", id="status-all")
-                        with RadioSet(id="status-filter-type"):
+                    yield RadioButton("Active only", id="status-active")
-                            yield RadioButton("Show All", id="status-all")
+                    yield RadioButton("Inactive only", id="status-inactive")
-                            yield RadioButton("Active Only", id="status-active")
-                            yield RadioButton("Inactive Only", id="status-inactive")
                     yield RadioButton("Custom", id="status-custom")
-
+                with Vertical(
-                        with Container(
+                    classes="filter-custom-options", id="status-custom-options"
-                            classes="filter-checkboxes", id="status-custom-options"
                 ):
                     yield Checkbox(
-                                "Show Active Entries", value=True, id="show-active"
+                        "Show active entries",
+                        value=True,
+                        id="show-active",
+                        compact=True,
                     )
                     yield Checkbox(
-                                "Show Inactive Entries", value=True, id="show-inactive"
+                        "Show inactive entries",
+                        value=True,
+                        id="show-inactive",
+                        compact=True,
                     )
 
-                # DNS type filtering section
+            with Vertical(classes="default-flex-section") as entry_type:
-                with Collapsible(title="Entry Type Filtering", collapsed=False):
+                entry_type.border_title = "Entry type"
-                    with Container(classes="filter-section"):
+                with RadioSet(id="type-filter-type", classes="default-radio-set"):
-                        yield Static(
+                    yield RadioButton("Show all", id="type-all")
-                            "Entry Type Filtering", classes="filter-section-title"
+                    yield RadioButton("DNS entries only", id="type-dns")
-                        )
+                    yield RadioButton("IP entries only", id="type-ip")
-                        with RadioSet(id="type-filter-type"):
-                            yield RadioButton("Show All", id="type-all")
-                            yield RadioButton("DNS Entries Only", id="type-dns")
-                            yield RadioButton("IP Entries Only", id="type-ip")
                     yield RadioButton("Custom", id="type-custom")
-
+                with Vertical(
-                        with Container(
+                    classes="filter-custom-options", id="type-custom-options"
-                            classes="filter-checkboxes", id="type-custom-options"
                 ):
                     yield Checkbox(
-                                "Show DNS Entries", value=True, id="show-dns"
+                        "Show DNS entries", value=True, id="show-dns", compact=True
+                    )
+                    yield Checkbox(
+                        "Show IP entries", value=True, id="show-ip", compact=True
                     )
-                            yield Checkbox("Show IP Entries", value=True, id="show-ip")
 
-                # DNS resolution status filtering section
+            with Vertical(classes="default-flex-section") as resolution:
-                with Collapsible(title="Resolution Status Filtering", collapsed=False):
+                resolution.border_title = "DNS resolution"
-                    with Container(classes="filter-section"):
+                with RadioSet(id="resolution-filter-type", classes="default-radio-set"):
-                        yield Static(
+                    yield RadioButton("Show all", id="resolution-all")
-                            "Resolution Status Filtering",
+                    yield RadioButton("Resolved only", id="resolution-resolved")
-                            classes="filter-section-title",
+                    yield RadioButton("Mismatches only", id="resolution-mismatch")
-                        )
-                        with RadioSet(id="resolution-filter-type"):
-                            yield RadioButton("Show All", id="resolution-all")
-                            yield RadioButton(
-                                "Resolved Only",
-                                id="resolution-resolved",
-                            )
-                            yield RadioButton(
-                                "Mismatches Only",
-                                id="resolution-mismatch",
-                            )
                     yield RadioButton("Custom", id="resolution-custom")
-
+                with Vertical(
-                        with Container(
+                    classes="filter-custom-options", id="resolution-custom-options"
-                            classes="filter-checkboxes", id="resolution-custom-options"
                 ):
                     yield Checkbox(
-                                "Show Resolved", value=True, id="show-resolved"
+                        "Show resolved", value=True, id="show-resolved", compact=True
                     )
                     yield Checkbox(
-                                "Show Unresolved", value=True, id="show-unresolved"
+                        "Show unresolved",
+                        value=True,
+                        id="show-unresolved",
+                        compact=True,
                     )
                     yield Checkbox(
-                                "Show Resolving", value=True, id="show-resolving"
+                        "Show resolving",
+                        value=True,
+                        id="show-resolving",
+                        compact=True,
                     )
-                            yield Checkbox("Show Failed", value=True, id="show-failed")
                     yield Checkbox(
-                                "Show Mismatched", value=True, id="show-mismatched"
+                        "Show failed", value=True, id="show-failed", compact=True
+                    )
+                    yield Checkbox(
+                        "Show mismatched",
+                        value=True,
+                        id="show-mismatched",
+                        compact=True,
                     )
 
-                # Search filtering section
+            with Vertical(classes="default-flex-section") as search:
-                with Collapsible(title="Search Filtering", collapsed=True):
+                search.border_title = "Search"
-                    with Container(classes="filter-section"):
-                        yield Static("Search Filtering", classes="filter-section-title")
-
-                        with Horizontal(classes="filter-input-row"):
-                            yield Label("Search term:", classes="filter-input-label")
                 yield Input(
-                                placeholder="Enter search term...",
+                    placeholder="Hostname, IP address, or comment",
                     value=self.current_options.search_term or "",
                     id="search-term",
-                                classes="filter-input",
+                    classes="filter-search-input",
                 )
-
+                with Vertical(classes="filter-custom-options"):
-                        with Container(classes="filter-checkboxes"):
                     yield Checkbox(
-                                "Search in hostnames", value=True, id="search-hostnames"
+                        "Search hostnames",
+                        value=True,
+                        id="search-hostnames",
+                        compact=True,
                     )
                     yield Checkbox(
-                                "Search in comments", value=True, id="search-comments"
+                        "Search comments",
+                        value=True,
+                        id="search-comments",
+                        compact=True,
                     )
                     yield Checkbox(
-                                "Search in IP addresses", value=True, id="search-ips"
+                        "Search IP addresses",
+                        value=True,
+                        id="search-ips",
+                        compact=True,
                     )
                     yield Checkbox(
                         "Case sensitive",
                         value=False,
                         id="search-case-sensitive",
+                        compact=True,
                     )
 
-                # Entry count display
+            with Horizontal(classes="filter-actions"):
-                yield Static("", id="count-display", classes="count-display")
+                yield Button(
-
+                    "Apply", id="apply", variant="primary", classes="default-button"
-            with Horizontal(id="filter-actions"):
+                )
-                yield Button("Apply", id="apply", variant="primary")
+                yield Button("Reset", id="reset", classes="default-button")
-                yield Button("Reset", id="reset")
+                yield Button("Cancel (ESC)", id="cancel", classes="default-button")
-                yield Button("Cancel", id="cancel")
 
     def _create_preset_select(self) -> Select:
         """Create the preset picker without selecting a preset by default."""
@@ -307,18 +200,90 @@ class FilterModal(ModalScreen[Optional[FilterOptions]]):
                 preset_options,
                 value=self.current_options.preset_name,
                 id="preset-select",
-                classes="preset-select",
+                classes="filter-preset-select",
+                compact=True,
             )
         return Select(
             preset_options,
             id="preset-select",
-            classes="preset-select",
+            classes="filter-preset-select",
+            compact=True,
         )
 
     def on_mount(self) -> None:
         """Initialize the modal with current options."""
         self._update_ui_from_options()
         self._update_count_display()
+        self.query_one("#preset-select", Select).focus()
+
+    def _focusable_control_ids(self) -> List[str]:
+        """Return the modal's tab order, omitting hidden custom controls."""
+        control_ids = [
+            "preset-select",
+            "load-preset",
+            "save-preset",
+            "delete-preset",
+            "status-filter-type",
+        ]
+        if self.query_one("#status-custom", RadioButton).value:
+            control_ids.extend(("show-active", "show-inactive"))
+
+        control_ids.append("type-filter-type")
+        if self.query_one("#type-custom", RadioButton).value:
+            control_ids.extend(("show-dns", "show-ip"))
+
+        control_ids.append("resolution-filter-type")
+        if self.query_one("#resolution-custom", RadioButton).value:
+            control_ids.extend(
+                (
+                    "show-resolved",
+                    "show-unresolved",
+                    "show-resolving",
+                    "show-failed",
+                    "show-mismatched",
+                )
+            )
+
+        control_ids.extend(
+            (
+                "search-term",
+                "search-hostnames",
+                "search-comments",
+                "search-ips",
+                "search-case-sensitive",
+                "apply",
+                "reset",
+                "cancel",
+            )
+        )
+        return control_ids
+
+    def action_focus_next(self) -> None:
+        """Move focus in form order, including controls below the scroll position."""
+        self._move_focus_in_form(1)
+
+    def action_focus_previous(self) -> None:
+        """Move focus backward in form order, including controls below the scroll position."""
+        self._move_focus_in_form(-1)
+
+    def _move_focus_in_form(self, direction: int) -> None:
+        """Focus the next or previous currently-visible form control."""
+        control_ids = self._focusable_control_ids()
+        focused_id = (
+            str(self.focused.id)
+            if self.focused is not None and self.focused.id is not None
+            else None
+        )
+        if focused_id is None:
+            focused_index = -1 if direction > 0 else 0
+        else:
+            try:
+                focused_index = control_ids.index(focused_id)
+            except ValueError:
+                focused_index = -1 if direction > 0 else 0
+
+        next_id = control_ids[(focused_index + direction) % len(control_ids)]
+        self.query_one(f"#{next_id}").focus()
 
     def _update_ui_from_options(self) -> None:
         """Update UI controls to reflect current options."""

src/hosts/tui/styles.py

@@ -313,6 +313,67 @@ ConfigModal {
 """
 )
 
+# Advanced Filter Modal CSS
+FILTER_MODAL_CSS = (
+    COMMON_CSS
+    + """
+FilterModal {
+    align: center middle;
+}
+
+.filter-container {
+    width: 80;
+    height: 28;
+    background: $surface;
+    border: thick $primary;
+    padding: 1;
+}
+
+.filter-title {
+    text-align: center;
+    text-style: bold;
+    color: $primary;
+    margin-bottom: 1;
+}
+
+.filter-count {
+    height: 1;
+    text-align: center;
+    color: $text-muted;
+    text-style: italic;
+    margin-bottom: 1;
+}
+
+.filter-preset-row {
+    height: 1;
+    align: left middle;
+}
+
+.filter-preset-select {
+    width: 1fr;
+    margin-right: 1;
+}
+
+.filter-custom-options {
+    height: auto;
+    margin: 0 2;
+}
+
+.filter-search-input {
+    height: 3;
+    width: 1fr;
+    margin: 0 2;
+}
+
+.filter-actions {
+    dock: bottom;
+    height: 3;
+    background: $surface;
+    align: center middle;
+}
+"""
+)
+
 # Save Confirmation Modal CSS
 SAVE_CONFIRMATION_MODAL_CSS = (
     COMMON_CSS

tests/test_app.py

@@ -72,6 +72,51 @@ async def test_filter_shortcut_opens_modal_and_applies_the_selected_filter():
     ]
 
 
+@pytest.mark.asyncio
+async def test_filter_shortcut_does_not_stack_filter_modals():
+    """Repeated filter shortcuts keep the existing filter modal in focus."""
+    app = app_with_filterable_entries()
+
+    async with app.run_test() as pilot:
+        await pilot.press("ctrl+f")
+        await pilot.pause()
+
+        await pilot.press("ctrl+f")
+        await pilot.press("ctrl+f")
+        await pilot.pause()
+
+        assert isinstance(app.screen, FilterModal)
+        assert sum(isinstance(screen, FilterModal) for screen in app.screen_stack) == 1
+
+
+@pytest.mark.asyncio
+async def test_filter_modal_tabs_through_controls_in_visual_order():
+    """The modal starts at Presets and tabs through the visible controls in order."""
+    app = app_with_filterable_entries()
+
+    async with app.run_test() as pilot:
+        await pilot.press("ctrl+f")
+        await pilot.pause()
+
+        assert app.focused is app.screen.query_one("#preset-select")
+        for expected_id in (
+            "load-preset",
+            "save-preset",
+            "delete-preset",
+            "status-filter-type",
+            "type-filter-type",
+            "resolution-filter-type",
+            "search-term",
+            "search-hostnames",
+            "search-comments",
+            "search-ips",
+            "search-case-sensitive",
+            "apply",
+        ):
+            await pilot.press("tab")
+            assert app.focused is app.screen.query_one(f"#{expected_id}")
+
+
 @pytest.mark.asyncio
 async def test_filter_reset_clears_filters_and_cancel_preserves_applied_filters():
     """Reset clears the form on Apply, while Cancel leaves the applied filter alone."""