table: Sort from the header cell and report one-row visible ranges

crates/component/src/table/delegate.rs

@@ -195,6 +195,9 @@ pub trait TableDelegate: Sized + 'static {
 
     /// Called when the visible range of the rows changed.
     ///
+    /// The range never ends past `rows_count`; a table whose rows drop to none
+    /// reports `0..0`.
+    ///
     /// NOTE: Make sure this method is fast, because it will be called frequently.
     ///
     /// This can used to handle some data update, to only update the visible rows.

crates/component/src/table/state.rs

@@ -819,7 +819,18 @@ where
         }
     }
 
-    fn on_col_head_click(&mut self, col_ix: usize, _: &mut Window, cx: &mut Context<Self>) {
+    /// A click on a sortable column's header, or on its sort icon, cycles the sort;
+    /// a click on any other header selects the column.
+    fn on_col_head_click(&mut self, col_ix: usize, window: &mut Window, cx: &mut Context<Self>) {
+        let column_sortable = self
+            .col_groups
+            .get(col_ix)
+            .is_some_and(|col_group| col_group.column.sort.is_some());
+        if self.sortable && column_sortable {
+            self.perform_sort(col_ix, window, cx);
+            return;
+        }
+
         if !self.col_selectable {
             return;
         }
@@ -1333,13 +1344,22 @@ where
     fn update_visible_range_if_need(
         &mut self,
         visible_range: Range<usize>,
+        items_count: usize,
         axis: Axis,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        // Skip when visible range is only 1 item.
+        // Skip when visible range is only 1 item, unless there is at most 1 item.
         // The visual_list will use first item to measure.
-        if visible_range.len() <= 1 {
+        if visible_range.len() <= 1 && items_count > 1 {
+            return;
+        }
+
+        // Stripe filler rows are rendered past the last row; never report them.
+        let end = visible_range.end.min(items_count);
+        let visible_range = visible_range.start.min(end)..end;
+        // A range wholly past the last item is stale; the list scrolls back next frame.
+        if visible_range.is_empty() && items_count > 0 {
             return;
         }
 
@@ -1620,9 +1640,6 @@ where
                 })
                 .hover(|this| this.bg(cx.theme().tokens.secondary).opacity(7.))
                 .active(|this| this.bg(cx.theme().tokens.secondary_active).opacity(1.))
-                .on_click(
-                    cx.listener(move |table, _, window, cx| table.perform_sort(col_ix, window, cx)),
-                )
                 .child(
                     Icon::new(icon)
                         .size_3()
@@ -2145,6 +2162,7 @@ where
                                     move |table, visible_range: Range<usize>, window, cx| {
                                         table.update_visible_range_if_need(
                                             visible_range.clone(),
+                                            columns_count.saturating_sub(left_columns_count),
                                             Axis::Horizontal,
                                             window,
                                             cx,
@@ -2459,6 +2477,8 @@ where
         };
 
         let empty_view = if rows_count == 0 {
+            // The rows list is not rendered, so report the empty range here.
+            self.update_visible_range_if_need(0..0, 0, Axis::Vertical, window, cx);
             Some(
                 div()
                     .size_full()
@@ -2520,6 +2540,7 @@ where
                                         );
                                         table.update_visible_range_if_need(
                                             visible_range.clone(),
+                                            rows_count,
                                             Axis::Vertical,
                                             window,
                                             cx,

crates/kit/tests/collections.rs

@@ -1,8 +1,10 @@
 mod common;
+use std::ops::Range;
+
 use gpui_kit::component::{
     IndexPath,
     list::{List, ListDelegate, ListItem, ListState},
-    table::{Column, DataTable, TableDelegate, TableSelection, TableState},
+    table::{Column, ColumnSort, DataTable, TableDelegate, TableSelection, TableState},
     tree::{Tree, TreeItem, TreeState},
 };
 use gpui_kit::test::TestWindowExt;
@@ -259,6 +261,158 @@ fn table_keyboard_leaves_rows_unselected_when_rows_are_not_selectable(cx: &mut T
     .unwrap();
 }
 
+/// A table whose first column sorts and whose second does not, recording what
+/// the table reports back to its delegate.
+struct TableProbe {
+    rows_count: usize,
+    sort_calls: Vec<(usize, ColumnSort)>,
+    visible_row_ranges: Vec<Range<usize>>,
+}
+impl TableProbe {
+    fn new(rows_count: usize) -> Self {
+        Self {
+            rows_count,
+            sort_calls: Vec::new(),
+            visible_row_ranges: Vec::new(),
+        }
+    }
+}
+impl TableDelegate for TableProbe {
+    fn columns_count(&self, _: &App) -> usize {
+        2
+    }
+    fn rows_count(&self, _: &App) -> usize {
+        self.rows_count
+    }
+    fn column(&self, col_ix: usize, _: &App) -> Column {
+        let column =
+            Column::new(format!("column-{col_ix}"), format!("Column {col_ix}")).width(px(180.));
+        if col_ix == 0 {
+            column.sortable()
+        } else {
+            column
+        }
+    }
+    fn render_td(
+        &mut self,
+        row_ix: usize,
+        col_ix: usize,
+        _: &mut Window,
+        _: &mut Context<TableState<Self>>,
+    ) -> impl IntoElement {
+        div().child(format!("{row_ix}:{col_ix}"))
+    }
+    fn perform_sort(
+        &mut self,
+        col_ix: usize,
+        sort: ColumnSort,
+        _: &mut Window,
+        _: &mut Context<TableState<Self>>,
+    ) {
+        self.sort_calls.push((col_ix, sort));
+    }
+    fn visible_rows_changed(
+        &mut self,
+        visible_range: Range<usize>,
+        _: &mut Window,
+        _: &mut Context<TableState<Self>>,
+    ) {
+        self.visible_row_ranges.push(visible_range);
+    }
+}
+struct TableProbeView {
+    table: Entity<TableState<TableProbe>>,
+    stripe: bool,
+}
+impl Render for TableProbeView {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        div()
+            .size_full()
+            .child(DataTable::new(&self.table).stripe(self.stripe))
+    }
+}
+fn open_table_probe(
+    cx: &mut TestAppContext,
+    rows_count: usize,
+    stripe: bool,
+) -> (gpui_kit::AnyWindowHandle, Entity<TableState<TableProbe>>) {
+    cx.update(gpui_kit::init);
+    let (handle, content) =
+        common::open_window(cx, Some(size(px(640.), px(320.))), |window, cx| {
+            cx.new(|cx| TableProbeView {
+                table: cx.new(|cx| TableState::new(TableProbe::new(rows_count), window, cx)),
+                stripe,
+            })
+        });
+    let table = cx
+        .update_window(handle.into(), |_, _, cx| content.read(cx).table.clone())
+        .unwrap();
+    (handle.into(), table)
+}
+
+#[gpui_kit::test]
+fn table_header_click_sorts_sortable_columns_and_selects_the_rest(cx: &mut TestAppContext) {
+    let (handle, table) = open_table_probe(cx, 3, false);
+    cx.update_window(handle, |_, window, cx| {
+        window.render_frame(cx);
+        for _ in 0..3 {
+            window.click(("col-header", 0usize), cx);
+        }
+        assert_eq!(
+            table.read(cx).delegate().sort_calls,
+            vec![
+                (0, ColumnSort::Descending),
+                (0, ColumnSort::Ascending),
+                (0, ColumnSort::Default),
+            ]
+        );
+        assert_eq!(table.read(cx).selected_col(), None);
+
+        window.click(("col-header", 1usize), cx);
+        assert_eq!(table.read(cx).selected_col(), Some(1));
+        assert_eq!(table.read(cx).delegate().sort_calls.len(), 3);
+    })
+    .unwrap();
+}
+
+#[gpui_kit::test]
+fn table_reports_one_row_and_empty_visible_ranges(cx: &mut TestAppContext) {
+    let (handle, table) = open_table_probe(cx, 1, false);
+    cx.update_window(handle, |_, window, cx| {
+        window.render_frame(cx);
+        assert_eq!(
+            table.read(cx).delegate().visible_row_ranges.last(),
+            Some(&(0..1))
+        );
+
+        table.update(cx, |table, cx| {
+            table.delegate_mut().rows_count = 0;
+            cx.notify();
+        });
+        window.render_frame(cx);
+        assert_eq!(
+            table.read(cx).delegate().visible_row_ranges.last(),
+            Some(&(0..0))
+        );
+    })
+    .unwrap();
+}
+
+#[gpui_kit::test]
+fn table_visible_range_stops_at_the_last_row_under_stripe_filler(cx: &mut TestAppContext) {
+    let (handle, table) = open_table_probe(cx, 3, true);
+    cx.update_window(handle, |_, window, cx| {
+        window.render_frame(cx);
+        let reported = table.read(cx).delegate().visible_row_ranges.clone();
+        assert!(!reported.is_empty());
+        assert!(
+            reported.iter().all(|range| range.end <= 3),
+            "reported past the last row: {reported:?}"
+        );
+    })
+    .unwrap();
+}
+
 struct Choices {
     confirmed: Vec<bool>,
 }

website/component/data-table.md

@@ -176,6 +176,8 @@ impl TableDelegate for LargeDataDelegate {
 
 ### Sorting Implementation
 
+Clicking the header of a sortable column cycles its sort: descending, ascending, then back to the default order. Clicking the header of a column that is not sortable selects the column.
+
 Implement sorting in your delegate:
 
 ```rust

website/zh-CN/component/data-table.md

@@ -152,6 +152,8 @@ impl TableDelegate for LargeDataDelegate {
 
 ## 排序
 
+点击可排序列的表头会依次切换排序：降序、升序，然后恢复默认顺序。点击不可排序列的表头会选中该列。
+
 排序逻辑需要由你的 `TableDelegate` 实现：
 
 ```rust