feat: improve vue node video preview loading and a11y#7558
Conversation
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthroughUpdates VideoPreview.vue component by restructuring accessibility interactions to an inner wrapper element, replacing the Changes
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)src/**/*.vue📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
**/*.vue📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,vue,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-12-11T03:55:51.755ZApplied to files:
📚 Learning: 2025-12-09T20:22:23.620ZApplied to files:
📚 Learning: 2025-12-09T03:49:52.828ZApplied to files:
📚 Learning: 2025-12-09T21:40:12.361ZApplied to files:
📚 Learning: 2025-12-11T12:25:15.470ZApplied to files:
⏰ 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 (6)
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 Build Status✅ Build completed successfully! ⏰ Completed at: 12/16/2025, 03:16:25 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/16/2025, 03:24:58 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
christian-byrne
added
needs-backport
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • ⚪ 0 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 992 kB (baseline 991 kB) • 🔴 +829 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 299 kB (baseline 299 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 184 kB (baseline 184 kB) • ⚪ 0 BReusable component library chunks
Status: 7 added / 7 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 3.18 kB (baseline 3.18 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.75 MB (baseline 3.75 MB) • ⚪ 0 BBundles that do not match a named category
Status: 19 added / 19 removed |
| width="100%" | ||
| height="100%" |
There was a problem hiding this comment.
Not for this PR
I still find it weird that we define the sizing in 3 ways
- width/height
- absolute + inset-0
- size-full
| const { t } = useI18n() | ||
| const nodeOutputStore = useNodeOutputStore() | ||
|
|
||
| const actionButtonClass = |
There was a problem hiding this comment.
I'll keep this in mind for the button variants.
| } | ||
|
|
||
| const handleFocusIn = () => { | ||
| isFocused.value = true |
There was a problem hiding this comment.
Nit: It really feels like we're using JavaScript to do what the DOM does well otherwise (hovered/focused state management)
There was a problem hiding this comment.
Could we do a group/focus-within to achieve the same goals?
## Summary Applies appropriate logic from below PRs (which affected image outputs/previews) to video previews - #7268: don't port the 250ms logic for videos, as it is not as relevant - videos typically take longer than 250ms and most browsers have a built-in loading state that will be displayed between component mount and video onloaded that we don't want to flash. Use the native video loaded event instead - #7252: apply to videos 1-for-1 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7558-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d365081eab4dcfeb1e62c553b) by [Unito](https://www.unito.io)
## Summary Applies appropriate logic from below PRs (which affected image outputs/previews) to video previews - #7268: don't port the 250ms logic for videos, as it is not as relevant - videos typically take longer than 250ms and most browsers have a built-in loading state that will be displayed between component mount and video onloaded that we don't want to flash. Use the native video loaded event instead - #7252: apply to videos 1-for-1 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7558-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d365081eab4dcfeb1e62c553b) by [Unito](https://www.unito.io)
|
@christian-byrne Successfully backported to #7561 |
|
@christian-byrne Successfully backported to #7562 |
github-actions
bot
removed
the
needs-backport
… a11y (#7561) Backport of #7558 to `core/1.35` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7561-backport-core-1-35-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d3650813284bdfdef8f3aef21) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
…d a11y (#7562) Backport of #7558 to `cloud/1.35` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7562-backport-cloud-1-35-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d365081b6ad0ef59a7ed547d1) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
## Summary Applies appropriate logic from below PRs (which affected image outputs/previews) to video previews - Comfy-Org#7268: don't port the 250ms logic for videos, as it is not as relevant - videos typically take longer than 250ms and most browsers have a built-in loading state that will be displayed between component mount and video onloaded that we don't want to flash. Use the native video loaded event instead - Comfy-Org#7252: apply to videos 1-for-1 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7558-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d365081eab4dcfeb1e62c553b) by [Unito](https://www.unito.io)
## Summary Applies appropriate logic from below PRs (which affected image outputs/previews) to video previews - #7268: don't port the 250ms logic for videos, as it is not as relevant - videos typically take longer than 250ms and most browsers have a built-in loading state that will be displayed between component mount and video onloaded that we don't want to flash. Use the native video loaded event instead - #7252: apply to videos 1-for-1 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7558-feat-improve-vue-node-video-preview-loading-and-a11y-2cb6d73d365081eab4dcfeb1e62c553b) by [Unito](https://www.unito.io)
5 participants
Summary
Applies appropriate logic from below PRs (which affected image outputs/previews) to video previews
┆Issue is synchronized with this Notion page by Unito