list: Fix selected and confirmed list item detail. (#520)

Closes #519 to fix popup_menu attempt to subtract with overflow bug.

crates/ui/src/list/list.rs

@@ -275,7 +275,8 @@ where
     }
 
     fn on_action_select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
-        if self.delegate.items_count(cx) == 0 {
+        let items_count = self.delegate.items_count(cx);
+        if items_count == 0 {
             return;
         }
 
@@ -283,7 +284,7 @@ where
         if selected_index > 0 {
             self.selected_index = Some(selected_index - 1);
         } else {
-            self.selected_index = Some(self.delegate.items_count(cx) - 1);
+            self.selected_index = Some(items_count - 1);
         }
 
         self.delegate.set_selected_index(self.selected_index, cx);
@@ -292,12 +293,13 @@ where
     }
 
     fn on_action_select_next(&mut self, _: &SelectNext, cx: &mut ViewContext<Self>) {
-        if self.delegate.items_count(cx) == 0 {
+        let items_count = self.delegate.items_count(cx);
+        if items_count == 0 {
             return;
         }
 
         if let Some(selected_index) = self.selected_index {
-            if selected_index < self.delegate.items_count(cx) - 1 {
+            if selected_index < items_count - 1 {
                 self.selected_index = Some(selected_index + 1);
             } else {
                 self.selected_index = Some(0);
@@ -312,27 +314,15 @@ where
     }
 
     fn render_list_item(&mut self, ix: usize, cx: &mut ViewContext<Self>) -> impl IntoElement {
+        let selected = self.selected_index == Some(ix);
+        let right_clicked = self.right_clicked_index == Some(ix);
+
         div()
             .id("list-item")
             .w_full()
             .relative()
             .children(self.delegate.render_item(ix, cx))
-            .when_some(self.selected_index, |this, selected_index| {
-                this.when(ix == selected_index, |this| {
-                    this.child(
-                        div()
-                            .absolute()
-                            .top(px(0.))
-                            .left(px(0.))
-                            .right(px(0.))
-                            .bottom(px(0.))
-                            .bg(cx.theme().list_active)
-                            .border_1()
-                            .border_color(cx.theme().list_active_border),
-                    )
-                })
-            })
-            .when(self.right_clicked_index == Some(ix), |this| {
+            .when(selected || right_clicked, |this| {
                 this.child(
                     div()
                         .absolute()
@@ -340,6 +330,7 @@ where
                         .left(px(0.))
                         .right(px(0.))
                         .bottom(px(0.))
+                        .when(selected, |this| this.bg(cx.theme().list_active))
                         .border_1()
                         .border_color(cx.theme().list_active_border),
                 )

crates/ui/src/list/list_item.rs

@@ -116,35 +116,28 @@ impl ParentElement for ListItem {
 
 impl RenderOnce for ListItem {
     fn render(self, cx: &mut WindowContext) -> impl IntoElement {
-        let is_active = self.selected || self.confirmed;
+        let is_active = self.confirmed || self.selected;
 
         self.base
             .text_color(cx.theme().foreground)
             .relative()
             .items_center()
             .justify_between()
-            .when_some(self.on_click, |this, on_click| {
-                if !self.disabled {
+            .when(is_active, |this| this.bg(cx.theme().list_active))
+            .when(!self.disabled, |this| {
+                this.when_some(self.on_click, |this, on_click| {
                     this.cursor_pointer()
                         .on_mouse_down(MouseButton::Left, move |_, cx| {
                             cx.stop_propagation();
                         })
                         .on_click(on_click)
-                } else {
-                    this
-                }
-            })
-            .when(is_active, |this| this.bg(cx.theme().list_active))
-            .when(!is_active && !self.disabled, |this| {
-                this.hover(|this| this.bg(cx.theme().list_hover))
-            })
-            // Mouse enter
-            .when_some(self.on_mouse_enter, |this, on_mouse_enter| {
-                if !self.disabled {
+                })
+                .when_some(self.on_mouse_enter, |this, on_mouse_enter| {
                     this.on_mouse_move(move |ev, cx| (on_mouse_enter)(ev, cx))
-                } else {
-                    this
-                }
+                })
+                .when(!is_active, |this| {
+                    this.hover(|this| this.bg(cx.theme().list_hover))
+                })
             })
             .child(
                 h_flex()

crates/ui/src/popup_menu.rs

@@ -389,9 +389,10 @@ impl PopupMenu {
     fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext<Self>) {
         let count = self.clickable_menu_items().count();
         if count > 0 {
+            let last_ix = count.saturating_sub(1);
             let ix = self
                 .selected_index
-                .map(|index| if index == count - 1 { 0 } else { index + 1 })
+                .map(|index| if index == last_ix { 0 } else { index + 1 })
                 .unwrap_or(0);
 
             self.selected_index = Some(ix);
@@ -402,10 +403,18 @@ impl PopupMenu {
     fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
         let count = self.clickable_menu_items().count();
         if count > 0 {
+            let last_ix = count.saturating_sub(1);
+
             let ix = self
                 .selected_index
-                .map(|index| if index == count - 1 { 0 } else { index - 1 })
-                .unwrap_or(count - 1);
+                .map(|index| {
+                    if index == last_ix {
+                        0
+                    } else {
+                        index.saturating_sub(1)
+                    }
+                })
+                .unwrap_or(last_ix);
             self.selected_index = Some(ix);
             cx.notify();
         }