Change RadioSet so it's less a container of widgets and more a widget

Initially we went with a RadioSet being a simple container of RadioButtons,
with the user navigating the RadioButtons like you would any other set of
widgets. This was fine but it became pretty clear pretty quickly that having
to tab through a non-trivial collection of buttons in a set to get to the
next widget wasn't ideal.

This commit, satisfying #2368, takes over the navigation of the buttons
within the container, makes the container itself a focusable widget, and
sets up some new bindings to allow a more natural and efficient interaction
with the set.

docs/examples/widgets/radio_button.py

@@ -20,7 +20,7 @@ class RadioChoicesApp(App[None]):
             yield RadioButton("Wing Commander")
 
     def on_mount(self) -> None:
-        self.query_one("#focus_me", RadioButton).focus()
+        self.query_one(RadioSet).focus()
 
 
 if __name__ == "__main__":

docs/examples/widgets/radio_set.py

@@ -9,7 +9,7 @@ class RadioChoicesApp(App[None]):
     def compose(self) -> ComposeResult:
         with Horizontal():
             # A RadioSet built up from RadioButtons.
-            with RadioSet():
+            with RadioSet(id="focus_me"):
                 yield RadioButton("Battlestar Galactica")
                 yield RadioButton("Dune 1984")
                 yield RadioButton("Dune 2021")
@@ -18,8 +18,7 @@ class RadioChoicesApp(App[None]):
                 yield RadioButton("Star Wars: A New Hope")
                 yield RadioButton("The Last Starfighter")
                 yield RadioButton(
-                    "Total Recall :backhand_index_pointing_right: :red_circle:",
+                    "Total Recall :backhand_index_pointing_right: :red_circle:"
-                    id="focus_me",
                 )
                 yield RadioButton("Wing Commander")
             # A RadioSet built up from a collection of strings.
@@ -36,7 +35,7 @@ class RadioChoicesApp(App[None]):
             )
 
     def on_mount(self) -> None:
-        self.query_one("#focus_me", RadioButton).focus()
+        self.query_one("#focus_me").focus()
 
 
 if __name__ == "__main__":

docs/examples/widgets/radio_set_changed.py

@@ -9,7 +9,7 @@ class RadioSetChangedApp(App[None]):
     def compose(self) -> ComposeResult:
         with VerticalScroll():
             with Horizontal():
-                with RadioSet():
+                with RadioSet(id="focus_me"):
                     yield RadioButton("Battlestar Galactica")
                     yield RadioButton("Dune 1984")
                     yield RadioButton("Dune 2021")
@@ -18,8 +18,7 @@ class RadioSetChangedApp(App[None]):
                     yield RadioButton("Star Wars: A New Hope")
                     yield RadioButton("The Last Starfighter")
                     yield RadioButton(
-                        "Total Recall :backhand_index_pointing_right: :red_circle:",
+                        "Total Recall :backhand_index_pointing_right: :red_circle:"
-                        id="focus_me",
                     )
                     yield RadioButton("Wing Commander")
             with Horizontal():
@@ -28,7 +27,7 @@ class RadioSetChangedApp(App[None]):
                 yield Label(id="index")
 
     def on_mount(self) -> None:
-        self.query_one("#focus_me", RadioButton).focus()
+        self.query_one(RadioSet).focus()
 
     def on_radio_set_changed(self, event: RadioSet.Changed) -> None:
         self.query_one("#pressed", Label).update(

src/textual/widgets/_radio_set.py

@@ -2,18 +2,19 @@
 
 from __future__ import annotations
 
-from typing import ClassVar
+from typing import ClassVar, Optional
 
 import rich.repr
 
 from ..binding import Binding, BindingType
 from ..containers import Container
-from ..events import Mount
+from ..events import Click, Mount
 from ..message import Message
+from ..reactive import var
 from ._radio_button import RadioButton
 
 
-class RadioSet(Container):
+class RadioSet(Container, can_focus=True, can_focus_children=False):
     """Widget for grouping a collection of radio buttons into a set.
 
     When a collection of [`RadioButton`][textual.widgets.RadioButton]s are
@@ -29,30 +30,47 @@ class RadioSet(Container):
         width: auto;
     }
 
-    RadioSet:focus-within {
+    RadioSet:focus {
         border: round $accent;
     }
 
     App.-light-mode RadioSet {
         border: round #CCC;
     }
+
+    /* The following rules/styles mimic similar ToggleButton:focus rules in
+     * ToggleButton. If those styles ever get updated, these should be too.
+     */
+
+    RadioSet:focus > RadioButton.-selected > .toggle--label {
+        text-style: underline;
+    }
+
+    RadioSet:focus ToggleButton.-selected > .toggle--button {
+        background: $foreground 25%;
+    }
+
+    RadioSet:focus > RadioButton.-on.-selected > .toggle--button {
+        background: $foreground 25%;
+    }
     """
 
     BINDINGS: ClassVar[list[BindingType]] = [
         Binding("down,right", "next_button", "", show=False),
-        Binding("shift+tab", "breakout_previous", "", show=False),
+        Binding("enter,space", "toggle", "Toggle", show=False),
-        Binding("tab", "breakout_next", "", show=False),
         Binding("up,left", "previous_button", "", show=False),
     ]
     """
     | Key(s) | Description |
     | :- | :- |
+    | enter, space | Toggle the currently-selected button. |
     | left, up | Select the previous radio button in the set. |
     | right, down | Select the next radio button in the set. |
-    | shift+tab | Move focus to the previous focusable widget relative to the set. |
-    | tab | Move focus to the next focusable widget relative to the set. |
     """
 
+    _selected: var[int | None] = var[Optional[int]](None)
+    """The index of the currently-selected radio button."""
+
     @rich.repr.auto
     class Changed(Message, bubble=True):
         """Posted when the pressed button in the set changes.
@@ -117,12 +135,26 @@ class RadioSet(Container):
     def _on_mount(self, _: Mount) -> None:
         """Perform some processing once mounted in the DOM."""
 
+        # If there are radio buttons, select the first one.
+        if self._nodes:
+            self._selected = 0
+
+        # Get all the buttons within us; we'll be doing a couple of things
+        # with that list.
+        buttons = list(self.query(RadioButton))
+
+        # RadioButtons can have focus, by default. But we're going to take
+        # that over and handle movement between them. So here we tell them
+        # all they can't focus.
+        for button in buttons:
+            button.can_focus = False
+
         # It's possible for the user to pass in a collection of radio
         # buttons, with more than one set to on; they shouldn't, but we
         # can't stop them. So here we check for that and, for want of a
         # better approach, we keep the first one on and turn all the others
         # off.
-        switched_on = [button for button in self.query(RadioButton) if button.value]
+        switched_on = [button for button in buttons if button.value]
         with self.prevent(RadioButton.Changed):
             for button in switched_on[1:]:
                 button.value = False
@@ -131,6 +163,11 @@ class RadioSet(Container):
         if switched_on:
             self._pressed_button = switched_on[0]
 
+    def watch__selected(self) -> None:
+        self.query(RadioButton).remove_class("-selected")
+        if self._selected is not None:
+            self._nodes[self._selected].add_class("-selected")
+
     def _on_radio_button_changed(self, event: RadioButton.Changed) -> None:
         """Respond to the value of a button in the set being changed.
 
@@ -160,6 +197,22 @@ class RadioSet(Container):
                 # We're being clicked off, we don't want that.
                 event.radio_button.value = True
 
+    def _on_radio_set_changed(self, event: RadioSet.Changed) -> None:
+        """Handle a change to which button in the set is pressed.
+
+        This handler ensures that, when a button is pressed, it's also the
+        selected button.
+        """
+        self._selected = event.index
+
+    async def _on_click(self, _: Click) -> None:
+        """Handle a click on or within the radio set.
+
+        This handler ensures that focus moves to the clicked radio set, even
+        if there's a click on one of the radio buttons it contains.
+        """
+        self.focus()
+
     @property
     def pressed_button(self) -> RadioButton | None:
         """The currently-pressed [`RadioButton`][textual.widgets.RadioButton], or `None` if none are pressed."""
@@ -179,31 +232,28 @@ class RadioSet(Container):
 
         Note that this will wrap around to the end if at the start.
         """
-        if self.children:
+        if self._nodes:
-            if self.screen.focused == self.children[0]:
+            if self._selected == 0:
-                self.screen.set_focus(self.children[-1])
+                self._selected = len(self.children) - 1
+            elif self._selected is None:
+                self._selected = 0
             else:
-                self.screen.focus_previous()
+                self._selected -= 1
 
     def action_next_button(self) -> None:
         """Navigate to the next button in the set.
 
         Note that this will wrap around to the start if at the end.
         """
-        if self.children:
+        if self._nodes:
-            if self.screen.focused == self.children[-1]:
+            if self._selected is None or self._selected == len(self._nodes) - 1:
-                self.screen.set_focus(self.children[0])
+                self._selected = 0
             else:
-                self.screen.focus_next()
+                self._selected += 1
 
-    def action_breakout_previous(self) -> None:
+    def action_toggle(self) -> None:
-        """Break out of the radio set to the previous widget in the focus chain."""
+        """Toggle the state of the currently-selected button."""
-        if self.children:
+        if self._selected is not None:
-            self.screen.set_focus(self.children[0])
+            button = self._nodes[self._selected]
-        self.screen.focus_previous()
+            assert isinstance(button, RadioButton)
-
+            button.toggle()
-    def action_breakout_next(self) -> None:
-        """Break out of the radio set to the next widget in the focus chain."""
-        if self.children:
-            self.screen.set_focus(self.children[-1])
-        self.screen.focus_next()

tests/toggles/test_radioset.py

@@ -57,13 +57,10 @@ async def test_radioset_inner_navigation():
     async with RadioSetApp().run_test() as pilot:
         assert pilot.app.screen.focused is None
         await pilot.press("tab")
-        assert (
-            pilot.app.screen.focused == pilot.app.query_one("#from_buttons").children[0]
-        )
         for key, landing in (("down", 1), ("up", 0), ("right", 1), ("left", 0)):
-            await pilot.press(key)
+            await pilot.press(key, "enter")
             assert (
-                pilot.app.screen.focused
+                pilot.app.query_one("#from_buttons", RadioSet).pressed_button
                 == pilot.app.query_one("#from_buttons").children[landing]
             )
 
@@ -73,11 +70,11 @@ async def test_radioset_breakout_navigation():
     async with RadioSetApp().run_test() as pilot:
         assert pilot.app.screen.focused is None
         await pilot.press("tab")
-        assert pilot.app.screen.focused.parent is pilot.app.query_one("#from_buttons")
+        assert pilot.app.screen.focused is pilot.app.query_one("#from_buttons")
         await pilot.press("tab")
-        assert pilot.app.screen.focused.parent is pilot.app.query_one("#from_strings")
+        assert pilot.app.screen.focused is pilot.app.query_one("#from_strings")
         await pilot.press("shift+tab")
-        assert pilot.app.screen.focused.parent is pilot.app.query_one("#from_buttons")
+        assert pilot.app.screen.focused is pilot.app.query_one("#from_buttons")
 
 
 class BadRadioSetApp(App[None]):