Skip to content

[Manager] Impletent “Install All” button #4196

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

Merged
merged 32 commits into from
Jun 18, 2025
Merged

Conversation

viva-jinyi
Copy link
Collaborator

@viva-jinyi viva-jinyi commented Jun 16, 2025

Summary

This PR implements the "Install All" functionality for missing node packs in both the missing nodes dialog and manager dialog's missing tab.

Behavior

  • Missing Nodes Dialog (LoadWorkflowWarning.vue):

    • Displays "Install All" button next to existing "Manager" button
    • Button only appears when there are missing node packs detected
    • One-click installation of all missing dependencies
  • Manager Dialog (ManagerDialogContent.vue):

    • Shows "Install All" button in top-right when viewing "missing" tab
    • Button integrates seamlessly with existing manager UI
    • Automatically detects and installs all missing packs from current workflow
  • Pack Detection Logic:

    • useMissingNodes composable automatically identifies missing node packs
    • Filters out already installed packs from workflow requirements
    • Handles loading states and error conditions gracefully

Components

  • PackInstallAllButton.vue: Reusable install all button component
  • useMissingNodes.ts: Core composable for missing pack detection and management
  • Enhanced existing components with install all integration
  • Updated styling in RegistrySearchBar.vue and PackActionButton.vue

Notes

  • i18n support added for install all button text

┆Issue is synchronized with this Notion page by Unito

viva-jinyi and others added 25 commits June 13, 2025 00:58
Co-authored-by: webfiltered <[email protected]>
@viva-jinyi viva-jinyi self-assigned this Jun 16, 2025
@viva-jinyi viva-jinyi requested a review from a team as a code owner June 16, 2025 08:38
Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work on this! Before merging, let's handle two things:

  1. Reuse check: Can we investigate whether the existing PackInstallButton.vue would work here instead of creating a new component?
  2. Rebase: Please rebase onto main when you get a chance.

Thanks!

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christian-byrne Do you have capacity to review & merge this? It could go straight into 1.23.0.

Edit: The timing of this was absolutely ridiculous. >.>

Copy link
Contributor

@christian-byrne christian-byrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@christian-byrne
Copy link
Contributor

To clean up the commit history, you can rebase instead of merging. Here's how:

git checkout migration/feat/installall
git rebase main

If there are conflicts, resolve them and run git rebase --continue. Once done, you'll need to force push: git push --force-with-lease

This will remove the merge commits and create a cleaner linear history.

@viva-jinyi viva-jinyi merged commit fa14ec5 into main Jun 18, 2025
10 checks passed
@viva-jinyi viva-jinyi deleted the migration/feat/installall branch June 18, 2025 01:52
comfy-jinyi pushed a commit that referenced this pull request Jun 18, 2025
Co-authored-by: github-actions <[email protected]>
Co-authored-by: filtered <[email protected]>
Co-authored-by: Christian Byrne <[email protected]>
Co-authored-by: Terry Jia <[email protected]>
Co-authored-by: comfy-waifu <[email protected]>
Co-authored-by: Comfy Org PR Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants