Skip to content

fix: scale timeline keyframe virtualization with scroll viewport height#9712

Open
tt-dmi wants to merge 1 commit into
HumanSignal:developfrom
tt-dmi:fix/timeline-keyframe-virtualization
Open

fix: scale timeline keyframe virtualization with scroll viewport height#9712
tt-dmi wants to merge 1 commit into
HumanSignal:developfrom
tt-dmi:fix/timeline-keyframe-virtualization

Conversation

@tt-dmi
Copy link
Copy Markdown

@tt-dmi tt-dmi commented Apr 29, 2026

Reason for change

This PR fixes a Video timeline rendering issue where keyframe diamonds stop appearing for
regions further down the timeline, even though the region rows themselves render.

Observed behavior:

  • on a VideoRectangle task with many regions, scrolling the timeline reveals rows whose
    labels render but whose keyframe diamond track is empty
  • increasing <Video timelineHeight="…" /> does not fix it — the cap is independent of
    the configured timeline height

This appears related to:

Root cause:

  • KeypointsVirtual (the inner virtualized list of keyframe rows) computed its render
    window from a hardcoded 165 px instead of the scroll container's actual height:
    const eIdx = clamp(sIdx + (Math.ceil(165 / height) - 1), 0, regions.length);
  • with height = 24, this caps the visible window at Math.ceil(165 / 24) - 1 = 6 rows +
    extra = 5 buffer ≈ 11 rows total
  • as soon as a task has more regions than that, rows outside the window receive
    renderable={false} from the parent and their keyframes are skipped

Solution:

  • read the scroll container's height from the existing useResizeObserver call (the same
    call already provides width for framesInView)
  • pass it through to KeypointsVirtual as viewHeight
  • use viewHeight instead of 165 when computing the render-window end index
  • keep ?? 165 as a fallback so the very first paint (before the ResizeObserver callback)
    reproduces the prior behavior exactly

What changed

In web/libs/editor/src/components/Timeline/Views/Frames/Frames.tsx:

  • destructure height alongside width from useResizeObserver(scrollable.current || [])
  • thread viewHeight={scrollableHeight ?? 165} into
  • add viewHeight: number to KeypointsVirtualProps and consume it in the bounds memo
    (Math.ceil(viewHeight / height) - 1)
  • add viewHeight to the bounds dependency list

Net diff: 6 insertions / 4 deletions in a single file.

Screenshots / Representation

Before — scrolled to the bottom of a 28-region video timeline; lower rows show labels but
no keyframes:

After — same task, same scroll position; every visible row renders its keyframes:

Rollout strategy

No feature flag.
This is a direct bug fix to the Video timeline render-window calculation.

Testing

Manual verification performed locally on Label Studio 1.23.0:

  • opened a VideoRectangle task with 28 regions
  • scrolled the timeline to the bottom
  • before the fix: the lower rows render labels but the keyframe diamond track is empty
    for ~17 of them
  • after the fix: every row visible in the scroll viewport renders its keyframes, for any
    timelineHeight

Additional validation performed:

  • confirmed that small region lists (≤ 11 rows) behave identically before and after —
    they always fit inside both the original 165 px window and the measured viewport
  • confirmed that the existing unit tests in
    web/libs/editor/src/components/Timeline/Views/Frames/Frames.test.tsx still pass — the
    useResizeObserver mock returns { width: 400 }, so scrollableHeight is undefined and the
    ?? 165 fallback path reproduces the prior behavior

Happy to add a unit test that asserts more rows become renderable as the observed height
grows — let me know if you'd like that in this PR or a follow-up.

Risks

Low.

Potential risks:

  • region rows now render their keyframes whenever they fall inside the actual scroll
    viewport, where previously they were silently capped at ~11; tasks with hundreds of
    regions and a tall timelineHeight will paint more keyframes per scroll position than they
    did before
  • the viewHeight value comes from a debounced ResizeObserver, so the very first paint
    uses the ?? 165 fallback for a single frame before the observer fires

Risk mitigations:

  • the change is contained to Frames.tsx (the parent) and the inner KeypointsVirtual — no
    public API or prop changes on
  • the ?? 165 fallback preserves the exact original behavior pre-measurement and in any
    environment without ResizeObserver
  • extra = 5 row over-render buffer is preserved

Reviewer notes

The most important logic is in:

  • the KeypointsVirtual bounds memo
  • the viewHeight prop wired from the parent

Please review:

  • whether scrollableHeight from useResizeObserver(scrollable.current || []) is the right
    measurement to feed virtualization (it's the same element already used for framesInView)
  • the ?? 165 fallback choice (preserves the previous magic constant for the first paint)

General notes

The 165 was a magic constant approximating ~6 visible rows × 24 px row height + slack. It
predates the useResizeObserver call that the same component now uses for horizontal
framesInView. This fix simply lets keyframe virtualization read from the same
observation.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 29, 2026

👷 Deploy request for label-studio-docs-new-theme pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 91dd872

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 29, 2026

👷 Deploy request for heartex-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 91dd872

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 29, 2026

Deploy Preview for label-studio-storybook ready!

Name Link
🔨 Latest commit 91dd872
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-storybook/deploys/69f1cb9084616f000828ff0a
😎 Deploy Preview https://deploy-preview-9712--label-studio-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 29, 2026

Deploy Preview for label-studio-playground ready!

Name Link
🔨 Latest commit 91dd872
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-playground/deploys/69f1cb908222ef00088a8ab7
😎 Deploy Preview https://deploy-preview-9712--label-studio-playground.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the fix label Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't see some regions in timeline when scrolling down

1 participant