fix: Refactor: Use Galleria lightbox for 'Open Image' instead of new tab (#9156)#9492
Draft
fix: Refactor: Use Galleria lightbox for 'Open Image' instead of new tab (#9156)#9492
Conversation
…tab (#9156) Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
📝 WalkthroughWalkthroughThis PR refactors the "Open Image" functionality to use a Galleria lightbox component instead of opening images in a new browser tab. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant ImageMenu as Image Menu Option
participant GalleryStore as Media Asset Gallery Store
participant GalleryComponent as ResultGallery Component
User->>ImageMenu: Click "Open Image"
ImageMenu->>GalleryStore: openUrl(url)
GalleryStore->>GalleryStore: Create ResultItemImpl with URL
GalleryStore->>GalleryStore: Update items & activeIndex
GalleryStore->>GalleryComponent: Notify state change
GalleryComponent->>GalleryComponent: Display lightbox with image
GalleryComponent->>User: Render gallery view
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ❌ 553 passed, 1 failed · 2 flaky❌ Failed Tests📊 Browser Reports
|
📦 Bundle: 4.53 MB gzip 🔴 +203 BDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 935 kB (baseline 935 kB) • 🔴 +438 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 76.6 kB (baseline 76.6 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 50.8 kB (baseline 50.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.75 MB (baseline 2.75 MB) • 🔴 +466 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 57.3 kB (baseline 57.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 8.88 MB (baseline 8.88 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.93 MB (baseline 7.93 MB) • ⚪ 0 BBundles that do not match a named category
Status: 51 added / 51 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-03-06T17:08:18.035Z",
"gitSha": "2152f5aed71c006d66f69e28032339d353215953",
"branch": "fix/coderabbit-issue-9156",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2034.0039999999817,
"styleRecalcs": 126,
"styleRecalcDurationMs": 23.323999999999998,
"layouts": 1,
"layoutDurationMs": 0.29600000000000004,
"taskDurationMs": 403.73,
"heapDeltaBytes": -1847176
},
{
"name": "canvas-idle",
"durationMs": 2020.0709999999162,
"styleRecalcs": 123,
"styleRecalcDurationMs": 17.787,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.34000000000003,
"heapDeltaBytes": -1713860
},
{
"name": "canvas-idle",
"durationMs": 2024.8699999999644,
"styleRecalcs": 123,
"styleRecalcDurationMs": 18.009999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 384.791,
"heapDeltaBytes": -1311028
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2040.2599999999893,
"styleRecalcs": 183,
"styleRecalcDurationMs": 51.52,
"layouts": 12,
"layoutDurationMs": 3.57,
"taskDurationMs": 1053.232,
"heapDeltaBytes": -2474012
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1861.14699999996,
"styleRecalcs": 170,
"styleRecalcDurationMs": 43.356,
"layouts": 12,
"layoutDurationMs": 3.18,
"taskDurationMs": 772.313,
"heapDeltaBytes": -2236208
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1785.1080000000366,
"styleRecalcs": 160,
"styleRecalcDurationMs": 39.354,
"layouts": 12,
"layoutDurationMs": 3.166,
"taskDurationMs": 741.752,
"heapDeltaBytes": -1278436
},
{
"name": "dom-widget-clipping",
"durationMs": 553.4959999999955,
"styleRecalcs": 40,
"styleRecalcDurationMs": 11.730999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 330.90999999999997,
"heapDeltaBytes": 6566192
},
{
"name": "dom-widget-clipping",
"durationMs": 533.7110000000393,
"styleRecalcs": 38,
"styleRecalcDurationMs": 12.24,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 342.762,
"heapDeltaBytes": 8341088
},
{
"name": "dom-widget-clipping",
"durationMs": 533.2889999999679,
"styleRecalcs": 39,
"styleRecalcDurationMs": 11.971999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 336.318,
"heapDeltaBytes": 6803456
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 609.1119999999819,
"styleRecalcs": 74,
"styleRecalcDurationMs": 13.585999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 414.446,
"heapDeltaBytes": -9189220
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 596.3660000001028,
"styleRecalcs": 73,
"styleRecalcDurationMs": 14.206,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 410.313,
"heapDeltaBytes": -7697020
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 597.0120000000634,
"styleRecalcs": 73,
"styleRecalcDurationMs": 13.994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 405.772,
"heapDeltaBytes": -9170732
},
{
"name": "subgraph-idle",
"durationMs": 2023.8870000000588,
"styleRecalcs": 124,
"styleRecalcDurationMs": 22.979,
"layouts": 1,
"layoutDurationMs": 0.25599999999999995,
"taskDurationMs": 391.259,
"heapDeltaBytes": -2224560
},
{
"name": "subgraph-idle",
"durationMs": 2026.004999999941,
"styleRecalcs": 124,
"styleRecalcDurationMs": 23.111,
"layouts": 1,
"layoutDurationMs": 0.272,
"taskDurationMs": 397.31899999999996,
"heapDeltaBytes": -766136
},
{
"name": "subgraph-idle",
"durationMs": 2005.174000000011,
"styleRecalcs": 121,
"styleRecalcDurationMs": 18.088,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 369.00100000000003,
"heapDeltaBytes": -1390276
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1680.8529999999564,
"styleRecalcs": 153,
"styleRecalcDurationMs": 40.236000000000004,
"layouts": 16,
"layoutDurationMs": 3.9290000000000003,
"taskDurationMs": 667.384,
"heapDeltaBytes": -4855036
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1680.1470000000336,
"styleRecalcs": 154,
"styleRecalcDurationMs": 42.096000000000004,
"layouts": 16,
"layoutDurationMs": 4.188,
"taskDurationMs": 681.057,
"heapDeltaBytes": -4953992
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1994.7409999999763,
"styleRecalcs": 172,
"styleRecalcDurationMs": 54.05199999999999,
"layouts": 16,
"layoutDurationMs": 3.874,
"taskDurationMs": 946.318,
"heapDeltaBytes": -3115080
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/assets/composables/useMediaAssetGalleryStore.ts`:
- Around line 40-47: The openUrl function currently constructs a ResultItemImpl
using filename: url.split('/').pop(), which fails for query-style Comfy URLs
like /view?filename=node-image.png&type=output; update openUrl to parse the URL
properly (use the URL constructor or URLSearchParams) to extract filename from
the query param "filename" first and fall back to the pathname basename if
absent, then pass that clean filename into new ResultItemImpl; also add a
regression test that calls openUrl (or the gallery-creation flow) with
"/view?filename=node-image.png&type=output" and asserts the stored
ResultItemImpl.filename equals "node-image.png".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b14cdd30-812b-4b1a-8d07-525b234e868c
📒 Files selected for processing (5)
src/composables/graph/useImageMenuOptions.tssrc/platform/assets/composables/useMediaAssetGalleryStore.test.tssrc/platform/assets/composables/useMediaAssetGalleryStore.tssrc/services/litegraphService.tssrc/views/GraphView.vue
Comment on lines
+40
to
+47
| const openUrl = (url: string) => { | ||
| const resultItem = new ResultItemImpl({ | ||
| filename: url.split('/').pop() ?? '', | ||
| subfolder: '', | ||
| type: 'output', | ||
| nodeId: '0', | ||
| mediaType: 'images' | ||
| }) |
Contributor
There was a problem hiding this comment.
Parse the filename from the URL instead of splitting the raw string.
Line 42 breaks for the real callers added in this PR: they pass Comfy /view?filename=... URLs, so the stored filename becomes something like view?filename=foo.png&type=output instead of foo.png. That leaks into ResultGallery as the image alt text and any other filename-based UI.
Suggested fix
const openUrl = (url: string) => {
+ const parsedUrl = new URL(url, window.location.origin)
+ const filenameFromPath =
+ decodeURIComponent(parsedUrl.pathname.split('/').pop() ?? '')
+
const resultItem = new ResultItemImpl({
- filename: url.split('/').pop() ?? '',
+ filename:
+ parsedUrl.searchParams.get('filename') ?? filenameFromPath,
subfolder: '',
type: 'output',
nodeId: '0',
mediaType: 'images'
})Please also add a regression test for a URL shaped like /view?filename=node-image.png&type=output.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const openUrl = (url: string) => { | |
| const resultItem = new ResultItemImpl({ | |
| filename: url.split('/').pop() ?? '', | |
| subfolder: '', | |
| type: 'output', | |
| nodeId: '0', | |
| mediaType: 'images' | |
| }) | |
| const openUrl = (url: string) => { | |
| const parsedUrl = new URL(url, window.location.origin) | |
| const filenameFromPath = | |
| decodeURIComponent(parsedUrl.pathname.split('/').pop() ?? '') | |
| const resultItem = new ResultItemImpl({ | |
| filename: | |
| parsedUrl.searchParams.get('filename') ?? filenameFromPath, | |
| subfolder: '', | |
| type: 'output', | |
| nodeId: '0', | |
| mediaType: 'images' | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/assets/composables/useMediaAssetGalleryStore.ts` around lines 40
- 47, The openUrl function currently constructs a ResultItemImpl using filename:
url.split('/').pop(), which fails for query-style Comfy URLs like
/view?filename=node-image.png&type=output; update openUrl to parse the URL
properly (use the URL constructor or URLSearchParams) to extract filename from
the query param "filename" first and fall back to the pathname basename if
absent, then pass that clean filename into new ResultItemImpl; also add a
regression test that calls openUrl (or the gallery-creation flow) with
"/view?filename=node-image.png&type=output" and asserts the stored
ResultItemImpl.filename equals "node-image.png".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9156
Summary
The "Open Image" context menu action on graph nodes previously opened images in a new browser tab using
openFileInNewTab. This was inconsistent with how images are viewed elsewhere in the app (e.g., the Assets sidebar), which uses the Galleria lightbox overlay. This change replaces the new-tab behavior with the existing Galleria lightbox component for a more consistent and integrated UX.Changes
src/platform/assets/composables/useMediaAssetGalleryStore.ts: AddedopenUrl(url)method that creates aResultItemImplwith an overridden URL getter, enabling the gallery to display an image from any URLsrc/composables/graph/useImageMenuOptions.ts: ReplacedopenFileInNewTabwithuseMediaAssetGalleryStore().openUrl()in theopenImagefunction; changed icon fromexternal-linktomaximizesrc/services/litegraphService.ts: Same replacement ofopenFileInNewTabwithuseMediaAssetGalleryStore().openUrl()for the litegraph "Open Image" callbacksrc/views/GraphView.vue: AddedResultGallerycomponent wired tomediaAssetGalleryStoreso the lightbox renders at the top-level viewsrc/platform/assets/composables/useMediaAssetGalleryStore.test.ts: Added tests for the newopenUrlmethodAutomated by coderabbit-fixer
┆Issue is synchronized with this Notion page by Unito