Fix LoadingList::selected() to return item reference

Rename the old selected() -> Option<usize> to selected_index() and add
a new selected() -> Option<&LoadingListItem<T>> for consistency with
other Envision components (Tabs, RadioGroup) that return item references
from their selected() method.

- Rename selected() to selected_index() (returns Option<usize>)
- Add selected() returning Option<&LoadingListItem<T>>
- Make selected_item() an alias for selected()
- Update all test call sites (~27) from selected() to selected_index()
- Add test_selected_returns_item test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: ryanoneill

src/component/loading_list/mod.rs

@@ -353,15 +353,43 @@ impl<T: Clone> LoadingListState<T> {
     }
 
     /// Returns the selected index.
-    pub fn selected(&self) -> Option<usize> {
+    pub fn selected_index(&self) -> Option<usize> {
         self.selected
     }
 
     /// Returns the selected item.
-    pub fn selected_item(&self) -> Option<&LoadingListItem<T>> {
+    ///
+    /// This is the canonical `selected()` method consistent with other Envision
+    /// components (Tabs, RadioGroup, etc.) that return `Option<&T>`.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// use envision::component::LoadingListState;
+    ///
+    /// #[derive(Clone)]
+    /// struct Task { name: String }
+    ///
+    /// let mut state = LoadingListState::with_items(
+    ///     vec![Task { name: "Build".to_string() }],
+    ///     |t| t.name.clone(),
+    /// );
+    /// assert!(state.selected().is_none());
+    ///
+    /// state.set_selected(Some(0));
+    /// assert_eq!(state.selected().unwrap().label(), "Build");
+    /// ```
+    pub fn selected(&self) -> Option<&LoadingListItem<T>> {
         self.selected.and_then(|i| self.items.get(i))
     }
 
+    /// Returns the selected item.
+    ///
+    /// Alias for [`selected()`](Self::selected).
+    pub fn selected_item(&self) -> Option<&LoadingListItem<T>> {
+        self.selected()
+    }
+
     /// Returns the selected item's data.
     pub fn selected_data(&self) -> Option<&T> {
         self.selected_item().map(|item| item.data())

src/component/loading_list/tests/component.rs

@@ -103,10 +103,10 @@ fn test_update_down() {
     let mut state = LoadingListState::with_items(items, |i| i.name.clone());
 
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(1));
+    assert_eq!(state.selected_index(), Some(1));
 }
 
 #[test]
@@ -116,7 +116,7 @@ fn test_update_down_wrap() {
     state.set_selected(Some(2)); // Last item
 
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0)); // Wraps
+    assert_eq!(state.selected_index(), Some(0)); // Wraps
 }
 
 #[test]
@@ -126,7 +126,7 @@ fn test_update_up() {
     state.set_selected(Some(2));
 
     LoadingList::update(&mut state, LoadingListMessage::Up);
-    assert_eq!(state.selected(), Some(1));
+    assert_eq!(state.selected_index(), Some(1));
 }
 
 #[test]
@@ -136,7 +136,7 @@ fn test_update_up_wrap() {
     state.set_selected(Some(0));
 
     LoadingList::update(&mut state, LoadingListMessage::Up);
-    assert_eq!(state.selected(), Some(2)); // Wraps
+    assert_eq!(state.selected_index(), Some(2)); // Wraps
 }
 
 #[test]
@@ -146,7 +146,7 @@ fn test_update_first() {
     state.set_selected(Some(2));
 
     LoadingList::update(&mut state, LoadingListMessage::First);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 }
 
 #[test]
@@ -155,7 +155,7 @@ fn test_update_last() {
     let mut state = LoadingListState::with_items(items, |i| i.name.clone());
 
     LoadingList::update(&mut state, LoadingListMessage::Last);
-    assert_eq!(state.selected(), Some(2));
+    assert_eq!(state.selected_index(), Some(2));
 }
 
 #[test]
@@ -323,7 +323,7 @@ fn test_update_set_items() {
     LoadingList::update(&mut state, LoadingListMessage::SetItems(items));
 
     assert_eq!(state.len(), 3);
-    assert!(state.selected().is_none()); // Selection cleared
+    assert!(state.selected_index().is_none()); // Selection cleared
     assert_eq!(state.items()[0].label(), "Item 1"); // Uses default labeling
 }
 
@@ -362,7 +362,7 @@ fn test_up_no_selection() {
     // No selection set
 
     let output = LoadingList::update(&mut state, LoadingListMessage::Up);
-    assert_eq!(state.selected(), Some(2)); // Goes to last item
+    assert_eq!(state.selected_index(), Some(2)); // Goes to last item
     assert!(matches!(
         output,
         Some(LoadingListOutput::SelectionChanged(2))
@@ -376,7 +376,7 @@ fn test_down_no_selection() {
     // No selection set
 
     let output = LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0)); // Goes to first item
+    assert_eq!(state.selected_index(), Some(0)); // Goes to first item
     assert!(matches!(
         output,
         Some(LoadingListOutput::SelectionChanged(0))
@@ -537,15 +537,15 @@ fn test_mixed_item_states_with_navigation() {
 
     // Navigate through all items regardless of state
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
     assert!(state.selected_item().unwrap().is_loading());
 
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(1));
+    assert_eq!(state.selected_index(), Some(1));
     assert!(state.selected_item().unwrap().is_error());
 
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(2));
+    assert_eq!(state.selected_index(), Some(2));
     assert!(state.selected_item().unwrap().is_ready());
 
     // Select works on items in any state
@@ -568,24 +568,24 @@ fn test_large_loading_list_navigation() {
 
     // Navigate through them (wrapping navigation, starts with no selection)
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 
     for _ in 0..50 {
         LoadingList::update(&mut state, LoadingListMessage::Down);
     }
-    assert_eq!(state.selected(), Some(50));
+    assert_eq!(state.selected_index(), Some(50));
 
     LoadingList::update(&mut state, LoadingListMessage::First);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 
     LoadingList::update(&mut state, LoadingListMessage::Last);
-    assert_eq!(state.selected(), Some(99));
+    assert_eq!(state.selected_index(), Some(99));
 
     // Down from last wraps to first
     LoadingList::update(&mut state, LoadingListMessage::Down);
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 
     // Up from first wraps to last
     LoadingList::update(&mut state, LoadingListMessage::Up);
-    assert_eq!(state.selected(), Some(99));
+    assert_eq!(state.selected_index(), Some(99));
 }

src/component/loading_list/tests/events.rs

@@ -74,7 +74,7 @@ fn test_dispatch_event() {
         output,
         Some(LoadingListOutput::SelectionChanged(0))
     ));
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 }
 
 // ========================================
@@ -97,15 +97,15 @@ fn test_instance_methods() {
         output,
         Some(LoadingListOutput::SelectionChanged(0))
     ));
-    assert_eq!(state.selected(), Some(0));
+    assert_eq!(state.selected_index(), Some(0));
 
     // instance dispatch_event
     let output = state.dispatch_event(&Event::key(KeyCode::Down));
     assert!(matches!(
         output,
         Some(LoadingListOutput::SelectionChanged(1))
     ));
-    assert_eq!(state.selected(), Some(1));
+    assert_eq!(state.selected_index(), Some(1));
 }
 
 // ========================================
@@ -152,7 +152,7 @@ fn test_disabled_prevents_navigation() {
 
     let output = LoadingList::<TestItem>::update(&mut state, LoadingListMessage::Down);
     assert_eq!(output, None);
-    assert_eq!(state.selected(), None);
+    assert_eq!(state.selected_index(), None);
 
     let output = LoadingList::<TestItem>::update(&mut state, LoadingListMessage::Up);
     assert_eq!(output, None);

src/component/loading_list/tests/state.rs

@@ -8,7 +8,7 @@ use super::*;
 fn test_state_new() {
     let state: LoadingListState<String> = LoadingListState::new();
     assert!(state.is_empty());
-    assert!(state.selected().is_none());
+    assert!(state.selected_index().is_none());
     assert!(state.show_indicators());
 }
 
@@ -83,23 +83,42 @@ fn test_state_counts() {
 }
 
 #[test]
-fn test_state_selected() {
+fn test_state_selected_index() {
     let items = make_items();
     let mut state = LoadingListState::with_items(items, |i| i.name.clone());
 
     state.set_selected(Some(1));
-    assert_eq!(state.selected(), Some(1));
+    assert_eq!(state.selected_index(), Some(1));
     assert_eq!(state.selected_item().unwrap().label(), "Item Two");
     assert_eq!(state.selected_data().unwrap().id, 2);
 }
 
+#[test]
+fn test_selected_returns_item() {
+    let items = make_items();
+    let mut state = LoadingListState::with_items(items, |i| i.name.clone());
+
+    // No selection returns None
+    assert!(state.selected().is_none());
+
+    // With selection returns the item
+    state.set_selected(Some(0));
+    let item = state.selected().unwrap();
+    assert_eq!(item.label(), "Item One");
+    assert_eq!(item.data().id, 1);
+
+    // selected() and selected_item() return the same thing
+    state.set_selected(Some(2));
+    assert_eq!(state.selected().unwrap().label(), state.selected_item().unwrap().label());
+}
+
 #[test]
 fn test_state_selected_clamped() {
     let items = make_items();
     let mut state = LoadingListState::with_items(items, |i| i.name.clone());
 
     state.set_selected(Some(100)); // Too high
-    assert_eq!(state.selected(), Some(2)); // Clamped to last
+    assert_eq!(state.selected_index(), Some(2)); // Clamped to last
 }
 
 #[test]
@@ -119,7 +138,7 @@ fn test_state_clear() {
 
     state.clear();
     assert!(state.is_empty());
-    assert!(state.selected().is_none());
+    assert!(state.selected_index().is_none());
 }
 
 #[test]