[feat] Add right-click context menu to MediaAssetCard #6844
Conversation
dosubot
bot
added
the
size:XL
📝 WalkthroughWalkthroughRefactors asset action UI from popover-based menus to a context-menu component, replaces MediaAssetMoreMenu with MediaAssetContextMenu, updates prop/event names and internal emits, exposes a click event on asset cards, adjusts composable signatures, adds a button size option, and updates locale action labels. Changes
Sequence DiagramsequenceDiagram
participant User
participant Card as MediaAssetCard
participant Menu as MediaAssetContextMenu
participant Actions as useMediaAssetActions
participant Parent
User->>Card: right-click / click ellipsis
activate Card
Card->>Card: handleContextMenu() / handle button click
Card->>Menu: show(event)
Card->>Parent: emit('context-menu-opened')
deactivate Card
User->>Menu: select menu item
activate Menu
Menu->>Actions: invoke action (download/add/open/export/copy/delete) with optional asset
activate Actions
Actions->>Actions: resolve targetAsset (param || mediaContext.asset)
deactivate Actions
alt delete chosen
Menu->>Parent: emit('asset-deleted')
else zoom or other
Menu->>Parent: emit('zoom') or perform action
end
Menu->>Menu: hide()
deactivate Menu
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/23/2025, 10:05:29 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/23/2025, 10:15:57 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
viva-jinyi
changed the title
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🟢 -3.67 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 943 kB (baseline 940 kB) • 🔴 +2.39 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 7.97 kB (baseline 7.97 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 306 kB (baseline 306 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 141 kB (baseline 141 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 5.7 MB (baseline 5.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.87 MB (baseline 3.87 MB) • ⚪ 0 BBundles that do not match a named category
Status: 18 added / 18 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/types/buttonTypes.ts (2)
16-23: Clarifyfull-widthpadding/text-size semantics for regular buttons
full-widthcurrently maps to only'w-full', unlikesm/mdwhich also define padding and text size. If you intend full‑width buttons to otherwise behave likemd, consider composing the classes, e.g. reusesizeClasses.mdor inline the same padding/text classes; otherwise these buttons will render with whatever padding/text styles are applied externally, which might be inconsistent.
67-74: Check icon full-width styling consistency (rounding & size)For icon buttons,
full-widthmaps to'w-full h-auto'whilesm/mdinclude explicitsize-*and!rounded-md. If you want full‑width icon buttons to look visually consistent with the existing ones, consider whether they should also enforce a fixed height and/or rounded corners; if the intent is a full‑bleed row without that styling, this is fine as is.src/components/sidebar/tabs/AssetsSidebarTab.vue (1)
82-94: Context menu ID coordination looks correctPassing
openContextMenuIdinto each card and updating it oncontext-menu-openedcleanly enforces a single open menu per tab. SinceopenContextMenuIdis only used as a signal for “another card opened its menu”, leaving it non‑null across tab/folder switches is harmless, but you could optionally reset it when changing views if you ever need stricter state tracking.Also applies to: 204-205
src/platform/assets/components/MediaAssetCard.vue (1)
19-21: Context menu integration on the card looks solid; verify implicit asset context for downloadsThe new flow on the card side is cohesive:
- Root
CardContaineremittingclickand handling@contextmenu.prevent="handleContextMenu"works well withAssetsSidebarTab’s@click="handleAssetSelect(item)"and per‑tabopenContextMenuId.- Wiring the ellipsis button to the same
handleContextMenukeeps left‑click and right‑click behavior consistent.- The dedicated
<MediaAssetContextMenu>child plus thewheneverwatcher that callscontextMenu.hide()whenopenContextMenuIdchanges correctly enforces a single open menu across cards without needing a “closed” event.One thing to confirm: the
@download="actions.downloadAsset()"handler in the media top component still relies onuseMediaAssetActionspicking up the current asset via injection. If this card is not always under aMediaAssetKeyprovider that matches the activeasset, that button will now silently no‑op (since the composable will early‑return whenmediaContextis null).If you want to decouple this from injected context entirely, an optional tweak is to pass the asset explicitly there, e.g.:
-@download="actions.downloadAsset()" +@download="asset && actions.downloadAsset(asset)"which will work regardless of any provider setup.
Also applies to: 62-77, 128-137, 184-200, 202-208, 211-212, 297-299, 341-352
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/sidebar/tabs/AssetsSidebarTab.vue(2 hunks)src/platform/assets/components/MediaAssetButtonDivider.vue(0 hunks)src/platform/assets/components/MediaAssetCard.vue(7 hunks)src/platform/assets/components/MediaAssetContextMenu.vue(1 hunks)src/platform/assets/components/MediaAssetMoreMenu.vue(0 hunks)src/platform/assets/composables/useMediaAssetActions.ts(6 hunks)src/types/buttonTypes.ts(3 hunks)
💤 Files with no reviewable changes (2)
- src/platform/assets/components/MediaAssetMoreMenu.vue
- src/platform/assets/components/MediaAssetButtonDivider.vue
🧰 Additional context used
🧬 Code graph analysis (1)
src/platform/assets/composables/useMediaAssetActions.ts (7)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(72-72)src/platform/assets/utils/assetUrlUtil.ts (1)
getAssetUrl(21-29)src/platform/assets/schemas/assetMetadataSchema.ts (1)
getOutputAssetMetadata(35-42)src/utils/loaderNodeUtil.ts (1)
detectNodeTypeFromFilename(21-38)src/platform/assets/utils/assetTypeUtil.ts (1)
getAssetType(19-24)src/utils/createAnnotatedPath.ts (1)
createAnnotatedPath(13-23)src/platform/workflow/utils/workflowExtractionUtil.ts (1)
extractWorkflowFromAsset(24-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (2)
src/types/buttonTypes.ts (1)
4-4: NewButtonSizevariant looks consistent with helpersAdding
'full-width'toButtonSizeis consistent with the mappings below and should be type‑safe across the codebase. Just be aware that any exhaustiveButtonSizehandling (e.g., switches) in other files may now need afull-widthbranch, but TypeScript should surface those at compile time.src/platform/assets/composables/useMediaAssetActions.ts (1)
61-63: The review comment's concern is incorrect; the call pattern is safeThe concern about
actions.downloadAsset()at MediaAssetCard.vue:42 silently failing due to missingmediaContextis unfounded. MediaAssetCard providesMediaAssetKeyat line 252 withasset: toRef(() => adaptedAsset.value), and the child component that emits@downloadis only rendered within thev-else-if="asset && adaptedAsset"conditional block (line 39). This guarantees thatmediaContext.asset.valueis always available and truthy when the handler is invoked.All other call sites in MediaAssetContextMenu.vue pass
assetexplicitly, so no issues exist elsewhere.Likely an incorrect or invalid review comment.
- Add i18n translations for all context menu labels - Fix separator logic to only add separators when there are items after them - Add missing translation keys to en/main.json 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
viva-jinyi
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
viva-jinyi
added
backport
christian-byrne
added
the
claude-review
github-actions
bot
removed
the
claude-review
Summary
Changes
MediaAssetContextMenu.vue- new component for context menuMediaAssetCard.vue- show context menu on right-click and more button clickMediaAssetMoreMenu.vue- consolidated into context menuMediaAssetButtonDivider.vue- unusedAssetsSidebarTab.vue- add context menu state managementuseMediaAssetActions.tsScreenshot
screen-capture.webm
🤖 Generated with Claude Code