context menu: Fix go to first element on context menu (#35875)

Alvaro Parker created 3 months ago

Closes #35873

Release Notes:

- Fixed bug where context menu doesn't circle back to the first item
when the last item is not selectable

Change summary

crates/ui/src/components/context_menu.rs | 29 +++++++++++++++++++++++--
1 file changed, 26 insertions(+), 3 deletions(-)

Detailed changes

crates/ui/src/components/context_menu.rs

@@ -679,18 +679,18 @@ impl ContextMenu {
             let next_index = ix + 1;
             if self.items.len() <= next_index {
                 self.select_first(&SelectFirst, window, cx);
+                return;
             } else {
                 for (ix, item) in self.items.iter().enumerate().skip(next_index) {
                     if item.is_selectable() {
                         self.select_index(ix, window, cx);
                         cx.notify();
-                        break;
+                        return;
                     }
                 }
             }
-        } else {
-            self.select_first(&SelectFirst, window, cx);
         }
+        self.select_first(&SelectFirst, window, cx);
     }
 
     pub fn select_previous(
@@ -1203,6 +1203,7 @@ mod tests {
                     .separator()
                     .separator()
                     .entry("Last entry", None, |_, _| {})
+                    .header("Last header")
             })
         });
 
@@ -1255,5 +1256,27 @@ mod tests {
                 "Should go back to previous selectable entry (first)"
             );
         });
+
+        context_menu.update_in(cx, |context_menu, window, cx| {
+            context_menu.select_first(&SelectFirst, window, cx);
+            assert_eq!(
+                Some(2),
+                context_menu.selected_index,
+                "Should start from the first selectable entry"
+            );
+
+            context_menu.select_previous(&SelectPrevious, window, cx);
+            assert_eq!(
+                Some(5),
+                context_menu.selected_index,
+                "Should wrap around to last selectable entry"
+            );
+            context_menu.select_next(&SelectNext, window, cx);
+            assert_eq!(
+                Some(2),
+                context_menu.selected_index,
+                "Should wrap around to first selectable entry"
+            );
+        });
     }
 }