fix(ui): smooth scrolling and preserve differential results#707
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Heuristic only — a main-side or flake label is a starting point, not a verdict. |
📝 WalkthroughWalkthroughPopup placement updates are coalesced per animation frame, differential-search submission state is propagated, and browser regression tests cover repeated scrolling and zero-match search responses. ChangesScrolling and differential behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…up-20260717 # Conflicts: # tests/ui-smoke.spec.ts
…up-20260717 # Conflicts: # tests/ui-smoke.spec.ts # tests/ui-stress.spec.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ui-smoke.spec.ts (1)
1344-1344: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire exactly one placement read.
Allowing zero reads means this test still passes if the scroll listener stops scheduling placement entirely. The twenty same-frame events should produce exactly one read.
Proposed fix
- expect(placementReads).toBeLessThanOrEqual(1); + expect(placementReads).toBe(1);🤖 Prompt for 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. In `@tests/ui-smoke.spec.ts` at line 1344, Update the placement read assertion in the scroll-listener test to require exactly one read from the twenty same-frame events, replacing the less-than-or-equal check with an exact equality assertion.
🤖 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.
Outside diff comments:
In `@tests/ui-smoke.spec.ts`:
- Line 1344: Update the placement read assertion in the scroll-listener test to
require exactly one read from the twenty same-frame events, replacing the
less-than-or-equal check with an exact equality assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 73ce138f-9db8-43d0-a352-6b258efc2882
📒 Files selected for processing (3)
docs/branch-review-ledger.mdtests/ui-smoke.spec.tstests/ui-tools.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/branch-review-ledger.md
- tests/ui-tools.spec.ts
Summary
Verification
npm run verify:pr-local— local aggregate wrapper was disrupted by concurrent-worktree resource contention; scoped stages and production build passed separatelynpm run verify:ui— full run reached 181/184; all three failures were isolated, hardened, and then passed repeatedly; hosted UI remains the exact-head gatenpm run verify:release— not run because it includes provider-backed gates outside this UI changeClinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Summary by CodeRabbit
Bug Fixes
Tests
Documentation