Skip to content

fix(virtual-scroll): record layout where the viewport is measured - #17739

Open
viktorkombov wants to merge 1 commit into
masterfrom
vkombov/fix-17688-23.0.x
Open

viktorkombov wants to merge 1 commit into
masterfrom
vkombov/fix-17688-23.0.x

Conversation

@viktorkombov

Copy link
Copy Markdown
Contributor

Closes #17688

Description

igx-virtual-scroll no longer emits stateChange while its host is detached after it has been laid out. When the host re-attaches, it emits the current window once, if that window differs from the last one reported.

Rendering is unchanged. A detached host still follows its reset offset, as #17662 needs for the ESF re-attach case; only the report is held back.

Motivation / Context

Regression from #17662 (2a54d50c3c), shipped in 22.2.0-rc.2.

Closing a remote-bound igx-combo or igx-simple-combo detaches the list. _syncScrollPosition follows the detached host's scrollTop of 0, so stateChange reported {0..k} and the combo emitted dataPreLoad for the first page after the drop-down had closed. Reopening restored the offset and requested the original page again. That is two extra requests per open/close cycle after any scroll. A page still in flight at close was cancelled by the consumer, so the reopened list stayed blank for one round trip.

Why not ignore the window in the combo while it is collapsed: that removes the requests, but the virtual scroll has still emitted {0..k}. Reopening at the top produces the same range, so no signal changes, the afterRenderEffect never re-runs, and the list stays blank with no request. IgxSimpleComboComponent.onClick does exactly this with an empty input. The re-attach report (_attachTick) covers that case.

Why _wasLaidOut: a plain _isLaidOut() check would also suppress the report made before the first open, while the list is not in the DOM yet, and the combo relies on that report.

Type of Change (check all that apply):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog
  • Skills/Agents

Component(s) / Area(s) Affected:

Virtual Scroll; Combo and Simple Combo (remote binding)

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

Six regression specs: three for the requests on close and reopen, three for reopening at the top. The last two assert the bound page and rendered rows, not the request count.

Spec Before Combo-side guard This PR
preserve loaded page - combo fail pass pass
keep request valid while closed - combo fail pass pass
preserve loaded page - simple-combo fail pass pass
scrolled to the top after reopening - combo fail pass pass
reopens at the top in the same task - combo pass fail pass
toggle button reopens a list - simple-combo pass fail pass

Suites: combo, simple-combo, virtual-scroll, drop-down, grid and tree-grid filtering - 874/874. Lint: 0 errors.

Also checked in headless Chrome by an automated script, using real input events through CDP and real Northwind pages in the simple-combo showcase. Escape and outside-click closes sent no request, and the toggle reopen requested index 0 and rendered rows 0-5. This was not a manual click-through.

Test Configuration:

  • Angular version: 22.2.0
  • Browser(s): Chrome 154 (headless)
  • OS: Windows 11

Screenshots / Recordings

None - no visual change.

Notes

  • No CHANGELOG entry. Against 22.1.x nothing changes for users, so this belongs in the rc.3 release notes.
  • Follow-ups, not in this PR:
    • totalItemCount shrinking below the current window leaves a blank list with no request. This is pre-existing.
    • The remote samples' onClosed assigns a page the combo did not request (samples repo).

🤖 Generated with Claude Code

Copilot AI left a comment

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.

Copilot review overview

🟢 Approval recommended

The implementation matches the reported regression and includes focused coverage for detached and reattached viewport behavior.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes detached virtual-scroll hosts from emitting transient window states that trigger redundant remote combo requests.

Changes:

  • Defers detached stateChange emissions until reattachment.
  • Adds regression coverage for combo reopening and pending requests.
  • Documents the revised emission behavior.
File Description
virtual-scroll.component.ts Tracks layout and deferred reattachment reports.
virtual-scroll.component.spec.ts Tests detached-host reporting.
virtual-scroll/​README.md Documents emission semantics.
simple-combo.component.spec.ts Covers remote page preservation and reopening.
combo.component.spec.ts Covers request lifecycle and restored windows.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Combo]: Remote-bound combo fires two extra data requests on every open/close cycle

3 participants