From 8f952f1b5812abe9fa88b5f60d5df1660c295c1a Mon Sep 17 00:00:00 2001
From: Finn Evers <finn@zed.dev>
Date: Tue, 29 Jul 2025 12:30:38 +0200
Subject: [PATCH] gpui: Ensure first tab index is selected on first focus
 (#35247)

This fixes an issue with tab indices where we would actually focus the
second focus handle on first focus instead of the first one. The test
was updated accordingly.

Release Notes:

- N/A

---------

Co-authored-by: Jason Lee <huacnlee@gmail.com>
---
 crates/gpui/examples/tab_stop.rs | 15 ++++++++++++---
 crates/gpui/src/tab_stop.rs      | 32 ++++++++++++++++++--------------
 2 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/crates/gpui/examples/tab_stop.rs b/crates/gpui/examples/tab_stop.rs
index 9c58b52a5e..1f6500f3e6 100644
--- a/crates/gpui/examples/tab_stop.rs
+++ b/crates/gpui/examples/tab_stop.rs
@@ -6,6 +6,7 @@ use gpui::{
 actions!(example, [Tab, TabPrev]);
 
 struct Example {
+    focus_handle: FocusHandle,
     items: Vec<FocusHandle>,
     message: SharedString,
 }
@@ -20,8 +21,11 @@ impl Example {
             cx.focus_handle().tab_index(2).tab_stop(true),
         ];
 
-        window.focus(items.first().unwrap());
+        let focus_handle = cx.focus_handle();
+        window.focus(&focus_handle);
+
         Self {
+            focus_handle,
             items,
             message: SharedString::from("Press `Tab`, `Shift-Tab` to switch focus."),
         }
@@ -40,6 +44,10 @@ impl Example {
 
 impl Render for Example {
     fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        fn tab_stop_style<T: Styled>(this: T) -> T {
+            this.border_3().border_color(gpui::blue())
+        }
+
         fn button(id: impl Into<ElementId>) -> Stateful<Div> {
             div()
                 .id(id)
@@ -52,12 +60,13 @@ impl Render for Example {
                 .border_color(gpui::black())
                 .bg(gpui::black())
                 .text_color(gpui::white())
-                .focus(|this| this.border_color(gpui::blue()))
+                .focus(tab_stop_style)
                 .shadow_sm()
         }
 
         div()
             .id("app")
+            .track_focus(&self.focus_handle)
             .on_action(cx.listener(Self::on_tab))
             .on_action(cx.listener(Self::on_tab_prev))
             .size_full()
@@ -86,7 +95,7 @@ impl Render for Example {
                             .border_color(gpui::black())
                             .when(
                                 item_handle.tab_stop && item_handle.is_focused(window),
-                                |this| this.border_color(gpui::blue()),
+                                tab_stop_style,
                             )
                             .map(|this| match item_handle.tab_stop {
                                 true => this
diff --git a/crates/gpui/src/tab_stop.rs b/crates/gpui/src/tab_stop.rs
index 1aa4cd6d9f..7dde42efed 100644
--- a/crates/gpui/src/tab_stop.rs
+++ b/crates/gpui/src/tab_stop.rs
@@ -32,20 +32,18 @@ impl TabHandles {
         self.handles.clear();
     }
 
-    fn current_index(&self, focused_id: Option<&FocusId>) -> usize {
-        self.handles
-            .iter()
-            .position(|h| Some(&h.id) == focused_id)
-            .unwrap_or_default()
+    fn current_index(&self, focused_id: Option<&FocusId>) -> Option<usize> {
+        self.handles.iter().position(|h| Some(&h.id) == focused_id)
     }
 
     pub(crate) fn next(&self, focused_id: Option<&FocusId>) -> Option<FocusHandle> {
-        let ix = self.current_index(focused_id);
-
-        let mut next_ix = ix + 1;
-        if next_ix + 1 > self.handles.len() {
-            next_ix = 0;
-        }
+        let next_ix = self
+            .current_index(focused_id)
+            .and_then(|ix| {
+                let next_ix = ix + 1;
+                (next_ix < self.handles.len()).then_some(next_ix)
+            })
+            .unwrap_or_default();
 
         if let Some(next_handle) = self.handles.get(next_ix) {
             Some(next_handle.clone())
@@ -55,7 +53,7 @@ impl TabHandles {
     }
 
     pub(crate) fn prev(&self, focused_id: Option<&FocusId>) -> Option<FocusHandle> {
-        let ix = self.current_index(focused_id);
+        let ix = self.current_index(focused_id).unwrap_or_default();
         let prev_ix;
         if ix == 0 {
             prev_ix = self.handles.len().saturating_sub(1);
@@ -108,8 +106,14 @@ mod tests {
             ]
         );
 
-        // next
-        assert_eq!(tab.next(None), Some(tab.handles[1].clone()));
+        // Select first tab index if no handle is currently focused.
+        assert_eq!(tab.next(None), Some(tab.handles[0].clone()));
+        // Select last tab index if no handle is currently focused.
+        assert_eq!(
+            tab.prev(None),
+            Some(tab.handles[tab.handles.len() - 1].clone())
+        );
+
         assert_eq!(
             tab.next(Some(&tab.handles[0].id)),
             Some(tab.handles[1].clone())