Skip to content

Commit f3eb538

Browse files
Copilotdsmmcken
andauthored
fix: keyboard navigation to last row/column enables sticky scrolling (#2562)
Fix keyboard shortcuts to end of table to apply stuckToBottom **Problem:** When using `Ctrl+End` or `Ctrl+ArrowDown` (mac: `Cmd` instead) to scroll to the bottom of a ticking table, the viewport moves to show the last row but does not enable `isStuckToBottom`. This means the table doesn't continue tracking new rows as they are added. **Solution:** Fixed `getTopForBottomVisible()` and `getLeftForRightVisible()` in `GridMetricCalculator` to account for scrollbar space, making them return the same values as `lastTop`/`lastLeft` when navigating to the last row/column which is used to determine if `isStuckToBottom`. **Changes:** - Modified `getTopForBottomVisible()` to deduct `scrollBarSize` from available height - Modified `getLeftForRightVisible()` to deduct `scrollBarSize` from available width - Added unit tests verifying sticky bottom behavior with keyboard shortcuts - Updated e2e test snapshots to reflect new expected visual behavior - All grid tests pass - Manually checked keyboard shortcut sticks to bottom of a time-table Keyboard navigation now properly triggers `isStuckToBottom` by scrolling to the maximum position that matches `lastTop`/`lastLeft`. Fixes #2361 --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: dsmmcken <[email protected]> Co-authored-by: Don McKenzie <[email protected]>
1 parent 8b23c8a commit f3eb538

File tree

5 files changed

+88
-4
lines changed

5 files changed

+88
-4
lines changed

packages/grid/src/Grid.test.tsx

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,4 +935,83 @@ describe('paste tests', () => {
935935
expect(model.setValues).toHaveBeenCalledTimes(1);
936936
});
937937
});
938+
939+
describe('sticky bottom behavior', () => {
940+
function makeGridWithStickyBottom(
941+
model: GridModel = new MockGridModel(),
942+
isStickyBottom = true
943+
): Grid {
944+
let ref: React.RefObject<Grid>;
945+
function GridWithRef() {
946+
ref = useRef<Grid>(null);
947+
return (
948+
<div>
949+
<Grid
950+
model={model}
951+
theme={defaultTheme}
952+
isStickyBottom={isStickyBottom}
953+
onSelectionChanged={jest.fn()}
954+
ref={ref}
955+
/>
956+
</div>
957+
);
958+
}
959+
render(<GridWithRef />);
960+
return ref!.current!;
961+
}
962+
963+
it('enables isStuckToBottom when navigating to last row with Ctrl+End', () => {
964+
const model = new MockGridModel();
965+
const { rowCount } = model;
966+
const component = makeGridWithStickyBottom(model, true);
967+
968+
// Start from the middle of the grid
969+
mouseClick(5, 5, component);
970+
971+
// Navigate to the last row with Ctrl+End
972+
end(component, { ctrlKey: true });
973+
974+
// Check that we're at the last row
975+
expect(component.state.selectionEndRow).toBe(rowCount - 1);
976+
977+
// Check that isStuckToBottom is now true
978+
expect(component.state.isStuckToBottom).toBe(true);
979+
});
980+
981+
it('enables isStuckToBottom when navigating to last row with Ctrl+ArrowDown', () => {
982+
const model = new MockGridModel();
983+
const { rowCount } = model;
984+
const component = makeGridWithStickyBottom(model, true);
985+
986+
// Start from the middle of the grid
987+
mouseClick(5, 5, component);
988+
989+
// Navigate to the last row with Ctrl+ArrowDown
990+
arrowDown(component, { ctrlKey: true });
991+
992+
// Check that we're at the last row
993+
expect(component.state.selectionEndRow).toBe(rowCount - 1);
994+
995+
// Check that isStuckToBottom is now true
996+
expect(component.state.isStuckToBottom).toBe(true);
997+
});
998+
999+
it('does not enable isStuckToBottom when isStickyBottom is false', () => {
1000+
const model = new MockGridModel();
1001+
const { rowCount } = model;
1002+
const component = makeGridWithStickyBottom(model, false);
1003+
1004+
// Start from the middle of the grid
1005+
mouseClick(5, 5, component);
1006+
1007+
// Navigate to the last row with Ctrl+End
1008+
end(component, { ctrlKey: true });
1009+
1010+
// Check that we're at the last row
1011+
expect(component.state.selectionEndRow).toBe(rowCount - 1);
1012+
1013+
// Check that isStuckToBottom is still false
1014+
expect(component.state.isStuckToBottom).toBe(false);
1015+
});
1016+
});
9381017
});

packages/grid/src/GridMetricCalculator.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -919,10 +919,13 @@ export class GridMetricCalculator {
919919
state: GridMetricState,
920920
bottomVisible: VisibleIndex
921921
): VisibleIndex {
922-
const { height } = state;
922+
const { height, theme } = state;
923+
const { scrollBarSize } = theme;
923924
const gridY = this.getGridY(state);
924925
const floatingBottomHeight = this.getFloatingBottomHeight(state);
925-
const availableHeight = height - gridY - floatingBottomHeight;
926+
// Account for scrollbar space to match lastTop calculation
927+
const availableHeight =
928+
height - gridY - floatingBottomHeight - scrollBarSize;
926929
return this.getLastTop(state, bottomVisible, availableHeight);
927930
}
928931

@@ -958,10 +961,12 @@ export class GridMetricCalculator {
958961
state: GridMetricState,
959962
rightVisible: VisibleIndex
960963
): VisibleIndex {
961-
const { width } = state;
964+
const { width, theme } = state;
965+
const { scrollBarSize } = theme;
962966
const gridX = this.getGridX(state);
963967
const floatingRightWidth = this.getFloatingRightWidth(state);
964-
const availableWidth = width - gridX - floatingRightWidth;
968+
// Account for scrollbar space to match lastLeft calculation
969+
const availableWidth = width - gridX - floatingRightWidth - scrollBarSize;
965970
return this.getLastLeft(state, rightVisible, availableWidth);
966971
}
967972

-1.11 KB
Loading
-1.87 KB
Loading
-623 Bytes
Loading

0 commit comments

Comments
 (0)