Linear: progressbar, tooltips, and output fixes#8250
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/22/2026, 11:05:35 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests:
|
📝 WalkthroughWalkthroughUpdates tooltip presentation in ModeToggle with localized labels and dynamic binding; adds responsive padding to LinearPreview's video element; substantially refactors OutputHistory with caching, asynchronous state management, output flattening, selection change emission, and progress bar UI. Changes
Possibly related PRs
Suggested reviewers
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 |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.6 kB (baseline 22.6 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 951 kB (baseline 948 kB) • 🔴 +2.5 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 439 kB (baseline 439 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 9 added / 9 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.83 kB (baseline 2.83 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.3 kB (baseline 33.3 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.16 MB (baseline 3.16 MB) • 🔴 +43 BStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 24 kB (baseline 24 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 6.36 MB (baseline 6.36 MB) • 🟢 -214 BBundles that do not match a named category
Status: 30 added / 30 removed |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/ModeToggle.vue (1)
23-34: Same accessibility fix needed for the Graph Mode button.Proposed fix
<Button v-tooltip="{ value: t('linearMode.graphMode'), showDelay: 300, hideDelay: 300 }" size="icon" + :aria-label="t('linearMode.graphMode')" :variant="canvasStore.linearMode ? 'secondary' : 'inverted'" `@click`="useCommandStore().execute('Comfy.ToggleLinear')" >
🤖 Fix all issues with AI agents
In `@src/components/sidebar/ModeToggle.vue`:
- Around line 11-22: The Button in ModeToggle.vue is icon-only and lacks an
aria-label; add an aria-label attribute to the Button using the same translated
string used in the tooltip (t('linearMode.linearMode')) so screen readers get
the button purpose, keeping the existing v-tooltip, size, variant, and `@click`
(references: Button component, t('linearMode.linearMode'),
canvasStore.linearMode, useCommandStore().execute('Comfy.ToggleLinear')).
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 105-146: The module-level outputsCache used by allOutputs grows
without bounds; update the cache strategy to avoid memory leaks by either
replacing outputsCache with a Map that enforces a max size/LRU eviction or by
clearing entries on component teardown. Specifically, change the outputsCache
usage in allOutputs (and where outputsCache is populated) to use a capped Map
with eviction logic, or import Vue's onUnmounted and clear/delete all keys from
outputsCache in an onUnmounted callback; ensure interactions with useAsyncState
and flattenNodeOutput remain the same and preserve storing the outputRef for
item.id.
- Around line 304-315: The progress bar elements lack ARIA semantics—add proper
accessibility attributes to convey progress to screen readers: on the visible
progress container (the element using progressBarContainerClass) set
role="progressbar" and provide aria-valuemin="0", aria-valuemax="100" and
aria-valuenow bound to the computed percentage (use
progressPercentStyle(totalPercent) value or a computed numeric totalPercent),
plus an aria-label or aria-labelledby describing the progress (e.g., "Output
generation progress"); for the layered bars (progressBarPrimaryClass and
progressBarSecondaryClass) ensure they do not duplicate roles (use
aria-hidden="true" if decorative) and consider adding aria-valuetext when you
need a localized percent string—use the existing progressPercentStyle and
currentNodePercent/totalPercent symbols to source the numeric values.
- Around line 125-146: The non-null assertion item!.id in function allOutputs
should be replaced with an explicit guard: capture the id early (e.g. const id =
item?.id) and use a conditional branch to ensure id is defined before caching
(outputsCache[id] = outputRef) and before the initial cache read; if id is
absent return [] immediately. Update all references in allOutputs (including the
initial if (item?.id && outputsCache[item.id]) check and the outputsCache
assignment) to use this explicit id guard to avoid the non-null assertion.
| function outputCount(item?: AssetItem) { | ||
| const user_metadata = getOutputAssetMetadata(item?.user_metadata) | ||
| if (!user_metadata?.allOutputs) return [] | ||
| return user_metadata?.outputCount ?? 0 | ||
| } | ||
|
|
||
| return user_metadata.allOutputs | ||
| const outputsCache: Record<string, MaybeRef<ResultItemImpl[]>> = {} | ||
|
|
||
| function flattenNodeOutput([nodeId, nodeOutput]: [ | ||
| string | number, | ||
| NodeExecutionOutput | ||
| ]): ResultItemImpl[] { | ||
| const knownOutputs: Record<string, ResultItem[]> = {} | ||
| if (nodeOutput.audio) knownOutputs.audio = nodeOutput.audio | ||
| if (nodeOutput.images) knownOutputs.images = nodeOutput.images | ||
| if (nodeOutput.video) knownOutputs.video = nodeOutput.video | ||
| if (nodeOutput.gifs) knownOutputs.gifs = nodeOutput.gifs as ResultItem[] | ||
| if (nodeOutput['3d']) knownOutputs['3d'] = nodeOutput['3d'] as ResultItem[] | ||
|
|
||
| return Object.entries(knownOutputs).flatMap(([mediaType, outputs]) => | ||
| outputs.map( | ||
| (output) => new ResultItemImpl({ ...output, mediaType, nodeId }) | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| function allOutputs(item?: AssetItem): MaybeRef<ResultItemImpl[]> { | ||
| if (item?.id && outputsCache[item.id]) return outputsCache[item.id] | ||
|
|
||
| const user_metadata = getOutputAssetMetadata(item?.user_metadata) | ||
| if (!user_metadata) return [] | ||
| if ( | ||
| user_metadata.allOutputs && | ||
| user_metadata.outputCount && | ||
| user_metadata.outputCount < user_metadata.allOutputs.length | ||
| ) | ||
| return user_metadata.allOutputs | ||
|
|
||
| const outputRef = useAsyncState( | ||
| getJobDetail(user_metadata.promptId).then((jobDetail) => { | ||
| if (!jobDetail?.outputs) return [] | ||
| return Object.entries(jobDetail.outputs).flatMap(flattenNodeOutput) | ||
| }), | ||
| [] | ||
| ).state | ||
| outputsCache[item!.id] = outputRef | ||
| return outputRef |
There was a problem hiding this comment.
This code seems like it might fit better in a centralized location or composable rather than in a component, WDYT?
There was a problem hiding this comment.
All the different history implementations are in need of a lot of polish/consolidation. The other implementations are close, but insufficient to fit the needs I have here. I think the correct move is to spend more time polishing those in order to be usable, but I couldn't justify the time that would be required to do so right now
That the implementation here isn't in a centralized location is for the best. I want to avoid my implementation becoming another competing standard.
The @/stores/queueStore implementation is closest, it just
- Doesn't support loading more assets for use with
useInfiniteScroll - Incorrectly casts outputs to
ResultItem[]- Which means text outputs aren't handled
- Is a little too needy in required inputs for construction of
TaskItemImpl
- Fixes only the first output being displayed in linear mode after the jobs migration - Fixes selected output no longer scrolling into view in history - Adds a progress bar indicator on running job <img width="113" height="102" alt="image" src="https://github.com/user-attachments/assets/ca684dbe-12c8-44aa-98f0-2985c0159156" /> - Moves linear toggle button to v-tooltip - Fixes placeholder sometimes continuing to display after a new output. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8250-Linear-progressbar-tooltips-and-output-fixes-2f06d73d365081ca9fa3ebf0e2516487) by [Unito](https://www.unito.io)
|
@AustinMroz Successfully backported to #8291 |
…#8291) Backport of #8250 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8291-backport-cloud-1-37-Linear-progressbar-tooltips-and-output-fixes-2f26d73d36508170a083eef8dfd1be50) by [Unito](https://www.unito.io) --------- Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: GitHub Action <action@github.com>
┆Issue is synchronized with this Notion page by Unito