-
Notifications
You must be signed in to change notification settings - Fork 491
fix: keep queue overlay expanded only #8215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoved hover-driven queue overlay behavior: deleted the QueueOverlayActive component and its test, removed TopMenuSection hover handlers and isTopMenuHovered state, and simplified QueueProgressOverlay to use only an optional expanded prop and two remaining overlay branches (expanded/empty). Changes
Sequence Diagram(s)mermaid 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 |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/22/2026, 10:36:51 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
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 — 939 kB (baseline 948 kB) • 🟢 -9.83 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) • 🟢 -52 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR simplifies the queue overlay by removing the mini hover states and keeping only the expanded view. The change is driven by the fact that progress information is now shown in the assets sidebar, making the mini passive overlay states redundant.
Changes:
- Removed mini queue overlay states (active and empty) in favor of a single expanded view
- Deleted QueueOverlayActive component and its test file
- Removed hover detection logic from TopMenuSection and QueueProgressOverlay
- Cleaned up unused composables and state management code
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/queue/QueueProgressOverlay.vue | Simplified to only render expanded state, removed hover logic, completion summary, and mini overlay components |
| src/components/queue/QueueOverlayActive.vue | Deleted - mini active overlay component no longer needed |
| src/components/queue/QueueOverlayActive.test.ts | Deleted - test file for removed component |
| src/components/TopMenuSection.vue | Removed hover tracking logic and menuHovered prop passing |
|
|
||
| import QueueOverlayActive from '@/components/queue/QueueOverlayActive.vue' | ||
| import QueueOverlayEmpty from '@/components/queue/QueueOverlayEmpty.vue' | ||
| import QueueOverlayExpanded from '@/components/queue/QueueOverlayExpanded.vue' |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The QueueOverlayEmpty component and its test file (QueueOverlayEmpty.vue and QueueOverlayEmpty.test.ts) appear to be orphaned by this change but were not deleted. Similar to QueueOverlayActive which was removed in this PR, QueueOverlayEmpty is no longer imported or used anywhere in the application code after removing the mini overlay states. Consider deleting these files along with the CompletionSummaryBanner component and its related files if they are no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too sure so I asked design, but I added it back anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/queue/QueueProgressOverlay.vue`:
- Around line 123-126: displayedJobGroups is a redundant computed that merely
returns groupedJobItems.value from useJobList(); remove the displayedJobGroups
computed, use groupedJobItems directly in the template and script, and delete
any references/exports of displayedJobGroups; update the destructuring from
useJobList() (or rename the destructured property) so the template continues to
receive the same reactive groupedJobItems without the extra computed wrapper.
| groupedJobItems | ||
| } = useJobList() | ||
|
|
||
| const displayedJobGroups = computed(() => groupedJobItems.value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider removing the redundant computed wrapper.
displayedJobGroups is a computed that simply returns groupedJobItems.value, which is already a computed from useJobList(). This adds unnecessary indirection. You can use groupedJobItems directly in the template.
♻️ Suggested simplification
const {
selectedJobTab,
selectedWorkflowFilter,
selectedSortMode,
hasFailedJobs,
filteredTasks,
- groupedJobItems
+ groupedJobItems: displayedJobGroups
} = useJobList()
-
-const displayedJobGroups = computed(() => groupedJobItems.value)This renames the destructured property directly, eliminating the extra computed wrapper while maintaining the same API for the template.
📝 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.
| groupedJobItems | |
| } = useJobList() | |
| const displayedJobGroups = computed(() => groupedJobItems.value) | |
| const { | |
| selectedJobTab, | |
| selectedWorkflowFilter, | |
| selectedSortMode, | |
| hasFailedJobs, | |
| filteredTasks, | |
| groupedJobItems: displayedJobGroups | |
| } = useJobList() |
🤖 Prompt for AI Agents
In `@src/components/queue/QueueProgressOverlay.vue` around lines 123 - 126,
displayedJobGroups is a redundant computed that merely returns
groupedJobItems.value from useJobList(); remove the displayedJobGroups computed,
use groupedJobItems directly in the template and script, and delete any
references/exports of displayedJobGroups; update the destructuring from
useJobList() (or rename the destructured property) so the template continues to
receive the same reactive groupedJobItems without the extra computed wrapper.
christian-byrne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you show the before and after please?
Added to PR description |
|
Completely forgot we are fully deleting the QPO. Making that change now. |
Remove the mini queue overlay states so only the expanded job list is available, since progress is shown in the assets sidebar.
Ready for merge now that the active jobs is now visible in the assets sidepanel, in both list and grid views.
Before
https://github.com/user-attachments/assets/e743e8d7-aa50-4446-8152-30e4b08204b0
After
https://github.com/user-attachments/assets/2fb9f361-3c26-43ae-8eca-97493ecea397
┆Issue is synchronized with this Notion page by Unito