Skip to content

[WIP] fix(look): compute exact timeline row height so rows stop shifting on scroll - #562

Merged
shinaBR2 merged 2 commits into
mainfrom
swo-634-look-timeline-row-estimate
Jul 26, 2026
Merged

[WIP] fix(look): compute exact timeline row height so rows stop shifting on scroll#562
shinaBR2 merged 2 commits into
mainfrom
swo-634-look-timeline-row-estimate

Conversation

@shinaBR2

@shinaBR2 shinaBR2 commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes SWO-634. Scrolling the Look photo timeline was janky: a row would show blank space, then blurry placeholder tiles at the wrong height, then snap/resize to its final square. That's a layout shift, and it contradicts the "zero layout shift" goal from SWO-607.

Cause: the virtualized grid estimated every photo row at a flat 220px, but a real row of square tiles is gridWidth ÷ columns tall — ~291px on desktop, ~174px on mobile. Every off-screen row was positioned with the wrong height, then jumped when measureElement reported the real one.

Fix: compute the exact row height from the live grid width instead of guessing. Every tile is a perfect square (aspectRatio: 1/1), so the height is fully determined by the content width, the column count, and the grid gap. A ref on the grid feeds its real width into estimateSize, so the estimate matches what's rendered and there's nothing left to correct. The virtualizer already re-runs the estimate on scroll and resize, so no observer/effect is needed — it tracks breakpoint and window changes on its own. 220px stays only as the pre-measurement first-paint fallback.

  • photoRowHeight() — new pure, unit-tested helper in home-page/utils.ts
  • container/index.tsx — reads the live grid width in estimateSize

Test plan

  • Verified live in the running Look app and in the Storybook PhotoTimelineContainer/Populated story: fast-scroll and window-resize no longer show rows resizing/snapping — rows land at final height on first paint.
  • photoRowHeight unit tests (gaps, single column, width-unknown fallback) — pass.
  • pnpm --filter ui typecheck, Biome lint on changed files, and the utils test suite all pass.

Manual check for a reviewer: open Look, scroll the timeline fast on both a wide window and a narrow (mobile-width) one — rows should not jump or resize as they enter.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved photo grid sizing during scrolling and window resizing.
    • Reduced visible snapping and resizing by using the actual available content width.
    • Added reliable fallback sizing during initial rendering.

… scroll

The virtualized timeline estimated every photo row at a flat 220px, but the
real square-tile row height is width/columns (~291px desktop, ~174px mobile).
Off-screen rows were mispositioned then snapped when measured — the blank →
blurry → resize jank on scroll. Read the live grid width from the DOM and
compute the exact height instead, so measureElement finds nothing to correct.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@shinaBR2 shinaBR2 self-assigned this Jul 26, 2026
@changeset-bot

changeset-bot Bot commented Jul 26, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: dd1da76

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
look dd1da76 Commit Preview URL

Branch Preview URL
Jul 26 2026, 10:13 AM

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@shinaBR2, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 235c237b-a2b4-45ac-8923-1e49961ef09a

📥 Commits

Reviewing files that changed from the base of the PR and between 96d6f34 and dd1da76.

📒 Files selected for processing (2)
  • packages/ui/src/look/home-page/container/index.tsx
  • packages/ui/src/look/home-page/utils.test.ts
📝 Walkthrough

Walkthrough

The home-page photo grid now calculates virtualized row heights from the rendered content width, column count, gap, and padding. A content ref supplies live width data, while unit tests cover calculation and fallback behavior.

Changes

Photo row sizing

Layer / File(s) Summary
Photo row height helper and tests
packages/ui/src/look/home-page/utils.ts, packages/ui/src/look/home-page/utils.test.ts
Adds and exports photoRowHeight, with tests for dimensions, column gaps, and zero-width fallback behavior.
Virtualizer width integration
packages/ui/src/look/home-page/container/index.tsx
Measures the content stack width and uses calculated photo row heights while retaining the header estimate as an initial fallback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • shinaBR2/sworld#540: Covers the same home-page virtualized timeline and related row-height utilities.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: computing exact timeline row heights to prevent scroll-induced shifting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 swo-634-look-timeline-row-estimate

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.

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #562      +/-   ##
==========================================
- Coverage   92.62%   92.61%   -0.01%     
==========================================
  Files         326      326              
  Lines        5735     5741       +6     
  Branches     1222     1223       +1     
==========================================
+ Hits         5312     5317       +5     
- Misses        423      424       +1     
Files with missing lines Coverage Δ
packages/ui/src/look/home-page/utils.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 973b8df...dd1da76. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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: 1

🤖 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 `@packages/ui/src/look/home-page/container/index.tsx`:
- Around line 70-90: Update the virtualizer setup around estimateSize and
virtualizer to invalidate cached row measurements whenever the layout inputs
contentWidth or columns change, using virtualizer.measure() or the established
equivalent. Ensure offscreen rows recalculate their heights after width or
column changes, and add a regression test covering this layout-driven
invalidation.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f7f5943-c571-4010-a8af-2da26e959e47

📥 Commits

Reviewing files that changed from the base of the PR and between 973b8df and 96d6f34.

📒 Files selected for processing (3)
  • packages/ui/src/look/home-page/container/index.tsx
  • packages/ui/src/look/home-page/utils.test.ts
  • packages/ui/src/look/home-page/utils.ts

Comment thread packages/ui/src/look/home-page/container/index.tsx Outdated
…hange

measureElement caches each row's measured height; a window resize or breakpoint
flip changes the true row height, but offscreen rows kept the stale cached value
until re-render, drifting scroll offsets. Observe the live grid width and call
virtualizer.measure() when the computed row height changes, dropping the cache
so every row re-estimates. Addresses CodeRabbit review on #562.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@shinaBR2
shinaBR2 merged commit 7728336 into main Jul 26, 2026
16 checks passed
@shinaBR2
shinaBR2 deleted the swo-634-look-timeline-row-estimate branch July 26, 2026 10:19
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