From d7cd0e54c24160cf7cd255f3b2a7c73e61e680a2 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 27 Dec 2024 18:41:27 +0800 Subject: [PATCH] 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 | 31 +++++++++++-------------------- crates/ui/src/list/list_item.rs | 27 ++++++++++----------------- crates/ui/src/popup_menu.rs | 15 ++++++++++++--- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/crates/ui/src/list/list.rs b/crates/ui/src/list/list.rs index eb3240ef..688f12df 100644 --- a/crates/ui/src/list/list.rs +++ b/crates/ui/src/list/list.rs @@ -275,7 +275,8 @@ where } fn on_action_select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext) { - 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) { - 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) -> 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), ) diff --git a/crates/ui/src/list/list_item.rs b/crates/ui/src/list/list_item.rs index a35a291a..48603aab 100644 --- a/crates/ui/src/list/list_item.rs +++ b/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() diff --git a/crates/ui/src/popup_menu.rs b/crates/ui/src/popup_menu.rs index 01b07273..40254102 100644 --- a/crates/ui/src/popup_menu.rs +++ b/crates/ui/src/popup_menu.rs @@ -389,9 +389,10 @@ impl PopupMenu { fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext) { 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) { 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(); }