Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3698,7 +3698,12 @@ export abstract class IgxGridBaseDirective implements GridType,
this.throttleTime$.pipe(
take(1),
switchMap(time => timer(time, this.throttleScheduler))
)
),
// `trailing: true` ensures the final settle position of a fast momentum
// scroll is processed; otherwise the last scroll events are dropped and the
// rows stay frozen at an intermediate startIndex while the scrollbar is at top.
// `leading: true` keeps the immediate response on scroll start.
{ leading: true, trailing: true }
Comment thread
Zneeky marked this conversation as resolved.
),
destructor
)
Expand Down
32 changes: 32 additions & 0 deletions projects/igniteui-angular/grids/grid/src/grid.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2161,6 +2161,38 @@ describe('IgxGrid Component Tests #grid', () => {

expect(grid.headerContainer.state.startIndex).toBeGreaterThan(0);
});

it('should settle at the top row when a fast momentum scroll ends at scrollTop 0 (throttle trailing edge)', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this test is placed under Zoneless rendering regressions? The changed logic is in the RxJS throttle configuration for scrollNotify, and the PR description states that the issue is independent of zone/zoneless change detection. Would it be more appropriate to place it under the general virtualization tests?

Comment on lines +2164 to +2165

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZonelessTallGridComponent, used by this test, auto-generates only three columns (ID, Name, and Value) inside a 600px-wide grid, so it does not render a horizontal scrollbar.

In my manual comparison against master, I could not reproduce the UI issue with only these three columns. It became reproducible at both vertical boundaries after adding enough columns to create horizontal overflow.

Because this test emits directly through grid.scrollNotify, it verifies the throttle configuration but bypasses the actual DOM scroll conditions. Could we add regression coverage using a fixture that guarantees horizontal overflow and asserts that it is present?

const fix = TestBed.createComponent(ZonelessTallGridComponent);
fix.detectChanges();
await fix.whenStable();
const grid = fix.componentInstance.grid;
const virtDir = grid.verticalScrollContainer;
const scrollEl = virtDir.getScroll();
const maxScroll = scrollEl.scrollHeight - scrollEl.clientHeight;
Comment thread
Zneeky marked this conversation as resolved.

// Move away from the top so the first rows are virtualized out of view.
grid.scrollNotify.next({ target: { scrollTop: maxScroll } });
await wait(50);
await fix.whenStable();
Comment thread
Zneeky marked this conversation as resolved.
expect(virtDir.state.startIndex).toBeGreaterThan(0);

// Simulate a fast momentum/inertia scroll back to the top: an intermediate
// position lands on the throttle's leading edge and the scrollTop = 0 settle
// arrives within the same throttle window, so it can only be delivered on the
// trailing edge. Feeding scrollNotify directly (instead of setting scrollTop)
// avoids the browser's async native scroll events - which all read the final
// scrollTop of 0 - from masking a dropped-trailing regression.
grid.scrollNotify.next({ target: { scrollTop: Math.round(maxScroll / 2) } });
grid.scrollNotify.next({ target: { scrollTop: 0 } });
await wait(50);
await fix.whenStable();

// Without the trailing edge the settle event is dropped and the grid stays
// frozen at an intermediate startIndex while the scrollbar sits at the top.
expect(virtDir.state.startIndex).toBe(0);
expect(grid.gridAPI.get_row_by_index(0)).toBeDefined();
});
});
});

Expand Down
Loading