Skip to content

Fix recurring mobile file-list scrolling (#370)#615

Merged
nitrobass24 merged 2 commits into
developfrom
fix/370-mobile-file-scroll
Jul 13, 2026
Merged

Fix recurring mobile file-list scrolling (#370)#615
nitrobass24 merged 2 commits into
developfrom
fix/370-mobile-file-scroll

Conversation

@nitrobass24

@nitrobass24 nitrobass24 commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • use native scrolling for the mobile file list where rows wrap and expand dynamically
  • retain CDK fixed-size virtualization on wider viewports for large-library performance
  • preserve dynamic viewport-height measurement across both rendering paths
  • disable browser scroll anchoring and smooth-scroll interference on the file viewport

Root cause

The CDK viewport was configured with a fixed 82px item size, but mobile rows are not fixed-height: they wrap and can expand for details and actions. The resulting offset drift causes rows to be skipped during scrolling. Angular/CDK patch updates address a separate scroll-anchoring regression but cannot correct this layout mismatch.

Addresses the recurrence reported in #370.

Validation

  • npx ng test --watch=false --include=src/app/pages/files/file-list.component.spec.ts (39 passed)
  • npx ng lint
  • npx ng build

The full local suite reached 562/569 tests under Node 26; the remaining seven failures are the existing localStorage test-environment incompatibility and are unrelated to this change.

Summary by CodeRabbit

  • New Features

    • Improved file-list scrolling on mobile devices with naturally sized rows and native scrolling.
    • Automatically switches between native scrolling on smaller screens and virtual scrolling on larger screens.
    • Adapts scrolling behavior when the viewport size changes.
  • Bug Fixes

    • Improved scrolling stability and prevented unwanted overscroll behavior.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 26a41f58-9eb9-404c-ac36-58dd0e7d9e20

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

FileListComponent now selects native scrolling on mobile and CDK virtual scrolling elsewhere, updates the mode when the viewport changes, adjusts viewport styling and height observation, and adds tests for both rendering paths.

Changes

Responsive file scrolling

Layer / File(s) Summary
Responsive scrolling state
src/angular/src/app/pages/files/file-list.component.ts
Adds a mobile media-query signal, synchronizes it with matchMedia changes, and removes the listener during teardown.
Conditional file-list rendering
src/angular/src/app/pages/files/file-list.component.html, src/angular/src/app/pages/files/file-list.component.scss
Renders naturally sized native rows on mobile, preserves virtual scrolling otherwise, and adds native scrolling and overscroll styles.
Viewport observation and validation
src/angular/src/app/pages/files/file-list.component.ts, src/angular/src/app/pages/files/file-list.component.spec.ts
Finds the active .file-viewport for chrome-height updates and tests both native and virtual scrolling modes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: fixing mobile file-list scrolling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/370-mobile-file-scroll

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/angular/src/app/pages/files/file-list.component.html`:
- Around line 25-61: Extract the duplicated app-file row markup and its bindings
into a shared ng-template, then render that template from both the native `@for`
branch and the cdkVirtualFor branch using NgTemplateOutlet. Add NgTemplateOutlet
to the component imports and pass each branch’s file and isEven values so both
paths preserve their current behavior and use the same row definition.

In `@src/angular/src/app/pages/files/file-list.component.spec.ts`:
- Around line 178-201: Extend the FileListComponent tests to mock
window.matchMedia with a fake MediaQueryList, verify the constructor uses the
expected query and registers the change listener, dispatch a change event to
confirm useNativeScrolling updates, and destroy the fixture to assert the same
listener is removed via ngOnDestroy. Keep the existing direct-signal rendering
assertions unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 18f801d6-0242-4d31-bd31-9d00c678d209

📥 Commits

Reviewing files that changed from the base of the PR and between fac64d7 and 4bb0f27.

📒 Files selected for processing (4)
  • src/angular/src/app/pages/files/file-list.component.html
  • src/angular/src/app/pages/files/file-list.component.scss
  • src/angular/src/app/pages/files/file-list.component.spec.ts
  • src/angular/src/app/pages/files/file-list.component.ts

Comment thread src/angular/src/app/pages/files/file-list.component.html
Comment thread src/angular/src/app/pages/files/file-list.component.spec.ts
@nitrobass24
nitrobass24 merged commit 6ed7ad1 into develop Jul 13, 2026
17 checks passed
@nitrobass24
nitrobass24 deleted the fix/370-mobile-file-scroll branch July 13, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant