feat(studio): sidebar asset card (unwired)#2210
Conversation
ukimsanov
force-pushed
the
studio-dnd/pr19-asset-card
branch
from
106e823 to
8f38050
Compare
ukimsanov
force-pushed
the
studio-dnd/pr18-nle-shell-assembly
branch
from
1b33229 to
fdda4e9
Compare
ukimsanov
force-pushed
the
studio-dnd/pr19-asset-card
branch
from
8f38050 to
c87f8d5
Compare
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Reviewed as part of the 25-PR NLE overhaul stack. Full stack review on #2205 (the keystone). No blockers on this PR. — Miga
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
R1 — Batch C (unwired chrome / assembly)
Verdict: 🟢 approve
Reviewed at c87f8d54. Focus per Phase-2 recon: sidebar AssetCard is presentational, verify unwired, sanity-check prop shape / consumer contract.
Unwired assertion — 🟢
Grepped packages/studio/src/**/*.{ts,tsx} at #2210 (top-of-Batch-C) for AssetCard imports. Zero consumers. The pre-existing AssetsTab.tsx still owns its inline asset-tile implementation; this file is a parallel extraction that will be swapped in at #2213.
The header comment is explicit about the extraction motivation: "Extracted from AssetsTab.tsx to keep that file under the 600-line CI gate."
Prop / store contract sanity — 🟢
AssetCard imports useAssetPreviewStore from @/utils/assetPreviewStore (added in #2204). Verified at #2210 head that:
assetPreviewStore.tsis present withpreviewAsset,previewProjectId,clearPreviewAsset,setPreviewAsset— the shapeAssetCardreads.AssetPreviewOverlay(from #2204) reads the same store, so store⇄reader contract is symmetric. When #2213 wires AssetsTab → AssetCard, the sidebar-writer / preview-reader pair activates atomically.
Other imports (VideoFrameThumbnail, AssetContextMenu, assetHelpers, mediaTypes, timelineAssetDrop, playerStore, assetClickBehavior) are all pre-existing utilities that AssetsTab already consumes today. No new dependency introduced.
Fallow lifecycle — 🟢
Added to .fallowrc.jsonc:
- unusedExports block:
sidebar/AssetCard.tsx— 1 entry. - complexity ignore block:
sidebar/AssetCard.tsx— 1 entry.
Both share the TEMP(studio-dnd) marker pointing at studio-dnd/pr22; removable in one motion.
Notes for Phase 3 cross-check
None. Clean unwired extraction.
R1 by Via
ukimsanov
force-pushed
the
studio-dnd/pr19-asset-card
branch
2 times, most recently
from
e3f7fa2 to
f90d3ee
Compare
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review — URL encoding gap: silent 404 for asset paths with special chars
Blocker-adjacent: AssetCard.tsx:174 builds the serve URL raw:
const serveUrl = `/api/projects/${projectId}/preview/${asset}`;No encodeURIComponent on any segment. The sibling AssetPreviewOverlay in the same PR (PR #2206 baseline, AssetPreviewOverlay.tsx:82) explicitly encodes:
previewAsset.split("/").map(encodeURIComponent).join("/")Impact: any asset path containing space, #, ?, %, non-ASCII, or + will 404 in:
- The card thumbnail (
<VideoFrameThumbnail src={serveUrl}>and<img src={serveUrl}>) useProbedDuration(which uses the same unencodedserveUrlat line 130)
While the overlay opens the same asset correctly. Same file, inconsistent encoding — will silently break for filenames like My Video.mp4 or non-ASCII asset names.
Fix: pipe asset through the same split("/").map(encodeURIComponent).join("/") helper that the overlay uses. Or extract to a shared encodePreviewPath() helper (the tail batch already has one at thumbnailUtils.ts:37-54 — reuse it).
Additional confirmed nit: useProbedDuration orphaned network fetch
RDJ's finding stands. AssetCard.tsx:40-63: cleanup only sets cancelled = true; the <video> element created via document.createElement("video") with preload="metadata" fires a network request when vid.src = src, and the cleanup never nulls vid.src or calls vid.load() to abort. On rapid scroll/filter in the sidebar, orphaned fetches queue up. Not corruption, real waste at scale.
Fix: in cleanup, call vid.removeAttribute("src") (or vid.src = "") + vid.load() before setting cancelled = true.
— Rames Jusso
ukimsanov
force-pushed
the
studio-dnd/pr18-nle-shell-assembly
branch
from
ddb8f80 to
49c25f9
Compare
ukimsanov
force-pushed
the
studio-dnd/pr19-asset-card
branch
from
f90d3ee to
3e255a1
Compare
What: AssetCard — the sidebar asset tile (thumbnail, hover preview, click policy, context menu incl. add-at-playhead), unwired. Why: standalone sidebar concept; AssetsTab adopts it in the app-shell swap. How: new file against the coexistence layer (assetHelpers, AssetContextMenu, assetClickBehavior, assetPreviewStore); one TEMP(studio-dnd) entry until AssetsTab wires it. Test plan: tsc --noEmit; fallow audit clean.
ukimsanov
force-pushed
the
studio-dnd/pr19-asset-card
branch
from
3e255a1 to
84523e1
Compare
ukimsanov
force-pushed
the
studio-dnd/pr18-nle-shell-assembly
branch
from
49c25f9 to
6fec739
Compare
4 participants
What
AssetCard — the sidebar asset tile (thumbnail, hover preview, click policy, context menu incl. add-at-playhead), unwired.
Why
standalone sidebar concept; AssetsTab adopts it in the app-shell swap.
How
new file against the coexistence layer (assetHelpers, AssetContextMenu, assetClickBehavior, assetPreviewStore); one TEMP(studio-dnd) entry until AssetsTab wires it.
Test plan
tsc --noEmit; fallow audit clean.
Stack position 19/25 — studio NLE overhaul (CapCut-parity timeline + canvas). Graphite manages bases; merge from #2192 upward. Temporary
TEMP(studio-dnd)fallow entries (unwired-component windows) are all removed by #2213 (app-shell swap), whose tree is byte-identical to the fully-verified integration branch.