Minor improvements to option list

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Parameter `animate` from `DataTable.move_cursor` was being ignored https://github.com/Textualize/textual/issues/3840
 - Fixed a crash if `DirectoryTree.show_root` was set before the DOM was fully available https://github.com/Textualize/textual/issues/2363
 - Live reloading of TCSS wouldn't apply CSS changes to screens under the top screen of the stack https://github.com/Textualize/textual/issues/3931
+- `SelectionList` option IDs are usable as soon as the widget is instantiated https://github.com/Textualize/textual/issues/3903
 
 
 ## [0.47.1] - 2023-01-05

src/textual/widgets/_option_list.py

@@ -351,7 +351,9 @@ def __init__(
         content that isn't an option.
         """
 
-        self._option_ids: dict[str, int] = {}
+        self._option_ids: dict[str, int] = {
+            option.id: index for index, option in enumerate(self._options) if option.id
+        }
         """A dictionary of option IDs and the option indexes they relate to."""
 
         self._lines: list[Line] = []
@@ -459,10 +461,6 @@ def _clear_content_tracking(self) -> None:
         """Clear down the content tracking information."""
         self._lines.clear()
         self._spans.clear()
-        # TODO: Having the option ID tracking be tied up with the main
-        # content tracking isn't necessary. Can possibly improve this a wee
-        # bit.
-        self._option_ids.clear()
 
     def _left_gutter_width(self) -> int:
         """Returns the size of any left gutter that should be taken into account.
@@ -501,7 +499,6 @@ def _refresh_content_tracking(self, force: bool = False) -> None:
         # Set up for doing less property access work inside the loop.
         lines_from = self.app.console.render_lines
         add_span = self._spans.append
-        option_ids = self._option_ids
         add_lines = self._lines.extend
 
         # Adjust the options for our purposes.
@@ -519,16 +516,18 @@ def _refresh_content_tracking(self, force: bool = False) -> None:
         # break out the individual lines that will be used to draw it, and
         # also set up the tracking of the actual options.
         line = 0
-        option = 0
+        option_index = 0
         padding = self.get_component_styles("option-list--option").padding
         for content in self._contents:
             if isinstance(content, Option):
                 # The content is an option, so render out the prompt and
                 # work out the lines needed to show it.
                 new_lines = [
                     Line(
-                        Strip(prompt_line).apply_style(Style(meta={"option": option})),
-                        option,
+                        Strip(prompt_line).apply_style(
+                            Style(meta={"option": option_index})
+                        ),
+                        option_index,
                     )
                     for prompt_line in lines_from(
                         Padding(content.prompt, padding) if padding else content.prompt,
@@ -537,11 +536,7 @@ def _refresh_content_tracking(self, force: bool = False) -> None:
                 ]
                 # Record the span information for the option.
                 add_span(OptionLineSpan(line, len(new_lines)))
-                if content.id is not None:
-                    # The option has an ID set, create a mapping from that
-                    # ID to the option so we can use it later.
-                    option_ids[content.id] = option
-                option += 1
+                option_index += 1
             else:
                 # The content isn't an option, so it must be a separator (if
                 # there were to be other non-option content for an option
@@ -602,9 +597,16 @@ def add_options(self, items: Iterable[NewOptionListContent]) -> Self:
             content = [self._make_content(item) for item in items]
             self._duplicate_id_check(content)
             self._contents.extend(content)
-            # Pull out the content that is genuine options and add them to the
-            # list of options.
-            self._options.extend([item for item in content if isinstance(item, Option)])
+            # Pull out the content that is genuine options. Add them to the
+            # list of options and map option IDs to their new indices.
+            new_options = [item for item in content if isinstance(item, Option)]
+            self._options.extend(new_options)
+            for new_option_index, new_option in enumerate(
+                new_options, start=len(self._options)
+            ):
+                if new_option.id:
+                    self._option_ids[new_option.id] = new_option_index
+
             self._refresh_content_tracking(force=True)
             self.refresh()
         return self
@@ -635,6 +637,12 @@ def _remove_option(self, index: int) -> None:
         option = self._options[index]
         del self._options[index]
         del self._contents[self._contents.index(option)]
+        # Decrement index of options after the one we just removed.
+        self._option_ids = {
+            option_id: option_index - 1 if option_index > index else option_index
+            for option_id, option_index in self._option_ids.items()
+            if option_index != index
+        }
         self._refresh_content_tracking(force=True)
         # Force a re-validation of the highlight.
         self.highlighted = self.highlighted
@@ -732,6 +740,7 @@ def clear_options(self) -> Self:
         """
         self._contents.clear()
         self._options.clear()
+        self._option_ids.clear()
         self.highlighted = None
         self._mouse_hovering_over = None
         self.virtual_size = Size(self.scrollable_content_region.width, 0)
@@ -966,7 +975,6 @@ def scroll_to_highlight(self, top: bool = False) -> None:
 
         Args:
             top: Scroll highlight to top of the list.
-
         """
         highlighted = self.highlighted
         if highlighted is None:
@@ -1070,11 +1078,11 @@ def _page(self, direction: Literal[-1, 1]) -> None:
                 # Looks like we've figured out the next option to jump to.
                 self.highlighted = target_option
 
-    def action_page_up(self):
+    def action_page_up(self) -> None:
         """Move the highlight up one page."""
         self._page(-1)
 
-    def action_page_down(self):
+    def action_page_down(self) -> None:
         """Move the highlight down one page."""
         self._page(1)
 

tests/css/test_css_reloading.py

@@ -59,6 +59,7 @@ async def test_css_reloading_applies_to_non_top_screen(monkeypatch) -> None:  #
 
         # Clear the CSS from the file.
         Path(CSS_PATH).write_text("/* This file has no rules intentionally. */\n")
+        await pilot.pause()
         await pilot.app._on_css_change()
         # Height should fall back to 1.
         assert first_label.styles.height is not None

tests/option_list/test_option_list_create.py

@@ -153,3 +153,11 @@ async def test_adding_multiple_duplicates_at_once() -> None:
                 ]
             )
         assert option_list.option_count == 5
+
+
+async def test_options_are_available_soon() -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/3903."""
+
+    option = Option("", id="some_id")
+    option_list = OptionList(option)
+    assert option_list.get_option("some_id") is option

tests/selection_list/test_selection_list_create.py

@@ -10,6 +10,7 @@
 
 import pytest
 from rich.text import Text
+
 from textual.app import App, ComposeResult
 from textual.widgets import SelectionList
 from textual.widgets.option_list import Option
@@ -98,9 +99,18 @@ async def test_add_non_selections() -> None:
         with pytest.raises(SelectionError):
             selections.add_option(("Nope", 0, False, 23))
 
+
 async def test_clear_options() -> None:
     """Clearing the options should also clear the selections."""
     async with SelectionListApp().run_test() as pilot:
         selections = pilot.app.query_one(SelectionList)
         selections.clear_options()
         assert selections.selected == []
+
+
+async def test_options_are_available_soon() -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/3903."""
+
+    selection = Selection("", 0, id="some_id")
+    selection_list = SelectionList[int](selection)
+    assert selection_list.get_option("some_id") is selection