Skip to content

fix(player): drop the empty transition label from the Loading assets card - #4327

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/player-loader-empty-rows
Sep 23, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
fix/player-loader-empty-rows

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

What changes for the user

The player's "Loading assets" card no longer shows a stray "transition" label under its progress bar.

Why it happened

The Loading assets card reuses the shader-transition loading panel. showAssetsLoading() resets that panel first, and reset() hid the transition-frame row but only emptied the transition row, so its "transition" label stayed on the card with no value next to it. The same bare label could show during a shader load that had not reported a count yet.

The fix

createShaderLoader() now returns the transition row too, and ShaderLoaderState sets both progress rows through one helper: a row is visible only while it has a value. reset() and update() both go through it, so the rule holds for the assets card and for shader loads.

Tests

  • New packages/player/src/shader-loader-state.test.ts (3 tests): no progress rows on the Loading assets card; the rows a shader load left behind are hidden when assets start loading; the transition row appears only once it has a count. All 3 fail on the previous code and pass now.
  • Player suite: 14 files, 397 tests passed. Typecheck, lint and format clean.

Review notes

An independent review found no blockers or majors. Two minor notes, left as they are:

  • The rows start visible when the panel is first built; every path that shows the panel resets or updates it first, so this is not reachable today.
  • The card keeps the rows' space (they are hidden with visibility, as the frame row already was), so there is a small blank area under the bar instead of the label.

Before / After

The card raised with the player's own showAssetsLoading() on a blank player, built from main (Before) and from this branch (After).

Before

Before: stray transition label

After

After: no label

…card

The Loading assets card reuses the shader-transition panel, and resetting it
hid the transition-frame row but not the transition row, so a bare
"transition" label stayed under the progress bar. Each progress row now shows
only while it has a value, which also covers a shader load that reports no
count yet.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 8de07ff8.

The root cause matches the PR body. The old reset() emptied transitionValue but never touched its row, while the frame row was already hidden. Since showAssetsLoading() goes through reset(), the bare "transition" label was left on the card. setRowValue applies one rule to both rows in both reset() and update(), so neither path can leave a label with no value next to it again. transitionStatus.row is the row createProgressRow already returns, so the new ShaderLoaderElements field is just exposing it.

update() does not change behaviour beyond visibility. transitionValue is the same ternary moved into a local, with "" as its fallback, and the frame row keeps its existing rule. The tests cover all three paths: a fresh assets card, a shader load followed by assets, and the transition row appearing only once it has a count.

Nothing blocking. Hiding with visibility keeps the blank space, as the body notes, and matches the frame row.

— Rames

@miguel-heygen
miguel-heygen merged commit 8f7e902 into main Sep 23, 2026
101 of 139 checks passed
@miguel-heygen
miguel-heygen deleted the fix/player-loader-empty-rows branch September 23, 2026 13:20
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.

2 participants