Clamp sub-column offset in horizontal scrolling to prevent overflow a⦠- #1463
Clamp sub-column offset in horizontal scrolling to prevent overflow aβ¦#1463Stelinkaa wants to merge 1 commit into
Conversation
β¦t the last page
There was a problem hiding this comment.
Pull request overview
Adjusts DataGrid horizontal scrolling to avoid accumulating a fractional βsub-columnβ offset once the grid reaches the last horizontal page, preventing visual overflow past the last columns.
Changes:
- Clamp
columnPixelOffsetto0when already at the last horizontal page. - Force the horizontal scrollbar position to the absolute end when at the last page (
maxScrollColumn + 1). - Clamp scrollbar-emitted scroll values so they canβt set the first visible column index beyond
maxScrollColumn.
Comments suppressed due to low confidence (1)
packages/web/src/datagrid/DataGridCore.svelte:2537
const fractionalCol = Math.min(e.detail, maxScrollColumn);removes the fractional range(maxScrollColumn, maxScrollColumn + 1)that previously allowed representing a sub-column offset on the last page. If the last scroll column is wider than the viewport (possible in narrow layouts), dragging the scrollbar to the right can no longer scroll within that last column. If the goal is only to prevent overscrolling into empty space, clampe.detailto a max value derived from the maximum allowed pixel offset at the end (e.g.,maxScrollColumn + maxOffset/lastColWidth) instead of clamping tomaxScrollColumnunconditionally.
const fractionalCol = Math.min(e.detail, maxScrollColumn);
const newIndex = Math.floor(fractionalCol);
const fraction = fractionalCol - newIndex;
const prevIndex = firstVisibleColumnScrollIndex;
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clamp sub-column offset when already at the last page (small delta that didn't trigger the loop) | ||
| if (columnPixelOffset > 0 && firstVisibleColumnScrollIndex >= maxScrollColumn) { | ||
| columnPixelOffset = 0; | ||
| } |
|
@claude-review |
2 similar comments
|
@claude-review |
|
@claude-review |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9034a43c76
βΉοΈ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (columnPixelOffset > 0 && firstVisibleColumnScrollIndex >= maxScrollColumn) { | ||
| columnPixelOffset = 0; |
There was a problem hiding this comment.
Allow offset for oversized last columns
When the final scrollable column is wider than gridScrollAreaWidth (for example in a narrow grid or after resizing a column), maxScrollColumn is the last column index, but horizontal scrolling still needs a positive columnPixelOffset/domTable.scrollLeft to reveal the column's right edge. This clamp resets that offset to 0 on every right-scroll once the index reaches maxScrollColumn, so the off-screen right side of the last column becomes unreachable.
Useful? React with πΒ / π.
|
@claude review |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. The three fixes work together correctly:
|
β¦t the last page