Merge pull request #4103 from davep/broken-optionlist-ids

Fix broken OptionList Option id mapping

Author: willmcgugan

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Fixed
+
+- Fixed broken `OptionList` `Option.id` mappings https://github.com/Textualize/textual/issues/4101
+
 ### Changed
 
 - Breaking change: keyboard navigation in `RadioSet`, `ListView`, `OptionList`, and `SelectionList`, no longer allows highlighting disabled items https://github.com/Textualize/textual/issues/3881

src/textual/widgets/_option_list.py

@@ -573,15 +573,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. Add them to the
-            # list of options and map option IDs to their new indices.
+            # Pull out the content that is genuine options, create any new
+            # ID mappings required, then add the new options to the option
+            # list.
             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._options.extend(new_options)
 
             self._refresh_content_tracking(force=True)
             self.refresh()

tests/option_list/test_option_list_id_stability.py

@@ -0,0 +1,22 @@
+"""Tests inspired by https://github.com/Textualize/textual/issues/4101"""
+
+from __future__ import annotations
+
+from textual.app import App, ComposeResult
+from textual.widgets import OptionList
+from textual.widgets.option_list import Option
+
+
+class OptionListApp(App[None]):
+    """Test option list application."""
+
+    def compose(self) -> ComposeResult:
+        yield OptionList()
+
+
+async def test_get_after_add() -> None:
+    """It should be possible to get an option by ID after adding."""
+    async with OptionListApp().run_test() as pilot:
+        option_list = pilot.app.query_one(OptionList)
+        option_list.add_option(Option("0", id="0"))
+        assert option_list.get_option("0").id == "0"