Skip to content

Conversation

@benceruleanlu
Copy link
Member

@benceruleanlu benceruleanlu commented Dec 15, 2025

Summary

Show a warning toast when leaving Server Config with pending changes, reminding users they must restart to apply changes.

Changes

  • What: Add a onBeforeUnmount toast in ServerConfigPanel when modifiedConfigs is non-empty and the user didn’t click Restart; add i18n strings.

Review Focus

  • Confirm the toast timing/conditions are correct (only fires on leaving the panel; suppressed when Restart is clicked).

Note

This is a stacked PR. (main <= #7478 <= #7479)

┆Issue is synchronized with this Notion page by Unito

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • backport

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/server-config-restart-required-toast

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 15, 2025

🎨 Storybook Build Status

Build completed successfully!

⏰ Completed at: 12/15/2025, 02:21:13 AM UTC

🔗 Links


🎉 Your Storybook is ready for review!

@github-actions
Copy link

github-actions bot commented Dec 15, 2025

🎭 Playwright Test Results

Some tests failed

⏰ Completed at: 12/15/2025, 02:31:07 AM UTC

📈 Summary

  • Total Tests: 504
  • Passed: 492 ✅
  • Failed: 1 ❌
  • Flaky: 2 ⚠️
  • Skipped: 9 ⏭️

📊 Test Reports by Browser

  • chromium: View Report • ✅ 480 / ❌ 1 / ⚠️ 2 / ⏭️ 9
  • chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
  • mobile-chrome: View Report • ✅ 9 / ❌ 0 / ⚠️ 0 / ⏭️ 0

🎉 Click on the links above to view detailed test results for each browser configuration.

@benceruleanlu
Copy link
Member Author

This one is more optional. There is already a component telling users that they need to restart.

@benceruleanlu benceruleanlu force-pushed the feat/server-config-restart-required-toast branch from 907a3a2 to 7b133c2 Compare December 15, 2025 02:19
@benceruleanlu
Copy link
Member Author

This PR is open because it's pretty small and simple, but I'm working on e2e desktop testing for this.

@benceruleanlu benceruleanlu marked this pull request as ready for review December 15, 2025 02:26
Copilot AI review requested due to automatic review settings December 15, 2025 02:26
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Dec 15, 2025
Copy link
Contributor

Copilot AI left a 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 adds a warning toast notification that reminds users to restart the application when they leave the Server Config panel with unsaved changes.

Key Changes:

  • Added onBeforeUnmount lifecycle hook to show a toast when leaving the panel with pending config changes
  • Implemented restartTriggered flag to suppress the toast when the user clicks the Restart button
  • Added i18n strings for the toast summary and detail messages

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/platform/settings/components/ServerConfigPanel.vue Adds toast notification logic with conditional display based on modified configs and restart state
src/locales/en/main.json Adds toast message strings for restart requirement notification

christian-byrne pushed a commit that referenced this pull request Dec 15, 2025
Adds a Desktop (Electron) Server-Config setting for
`--enable-manager-legacy-ui` so users can opt into ComfyUI-Manager’s
legacy UI.

- Adds `enable-manager-legacy-ui` to `SERVER_CONFIG_ITEMS`
- Adds EN i18n label + tooltip

Note: this PR only adds the setting/flag wiring; it does not change
restart behavior in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7478-feat-server-config-add-legacy-manager-UI-toggle-2ca6d73d365081a79bb2c376506f5346)
by [Unito](https://www.unito.io)


> [!NOTE]
> This is a stacked PR. (main <=
#7478 <=
#7479)
Base automatically changed from feat/server-config-legacy-manager-ui to main December 15, 2025 02:40
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Dec 15, 2025
github-actions bot pushed a commit that referenced this pull request Dec 15, 2025
Adds a Desktop (Electron) Server-Config setting for
`--enable-manager-legacy-ui` so users can opt into ComfyUI-Manager’s
legacy UI.

- Adds `enable-manager-legacy-ui` to `SERVER_CONFIG_ITEMS`
- Adds EN i18n label + tooltip

Note: this PR only adds the setting/flag wiring; it does not change
restart behavior in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7478-feat-server-config-add-legacy-manager-UI-toggle-2ca6d73d365081a79bb2c376506f5346)
by [Unito](https://www.unito.io)


> [!NOTE]
> This is a stacked PR. (main <=
#7478 <=
#7479)
github-actions bot pushed a commit that referenced this pull request Dec 15, 2025
Adds a Desktop (Electron) Server-Config setting for
`--enable-manager-legacy-ui` so users can opt into ComfyUI-Manager’s
legacy UI.

- Adds `enable-manager-legacy-ui` to `SERVER_CONFIG_ITEMS`
- Adds EN i18n label + tooltip

Note: this PR only adds the setting/flag wiring; it does not change
restart behavior in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7478-feat-server-config-add-legacy-manager-UI-toggle-2ca6d73d365081a79bb2c376506f5346)
by [Unito](https://www.unito.io)


> [!NOTE]
> This is a stacked PR. (main <=
#7478 <=
#7479)
}

const restartApp = async () => {
restartTriggered = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the toast say that a restart is required while the app is already restarting?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, restartTriggered should be guarding that

@christian-byrne christian-byrne merged commit 93195d3 into main Dec 16, 2025
40 of 41 checks passed
@christian-byrne christian-byrne deleted the feat/server-config-restart-required-toast branch December 16, 2025 13:57
Enferlain pushed a commit to Enferlain/ComfyUI_frontend that referenced this pull request Dec 18, 2025
Adds a Desktop (Electron) Server-Config setting for
`--enable-manager-legacy-ui` so users can opt into ComfyUI-Manager’s
legacy UI.

- Adds `enable-manager-legacy-ui` to `SERVER_CONFIG_ITEMS`
- Adds EN i18n label + tooltip

Note: this PR only adds the setting/flag wiring; it does not change
restart behavior in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7478-feat-server-config-add-legacy-manager-UI-toggle-2ca6d73d365081a79bb2c376506f5346)
by [Unito](https://www.unito.io)


> [!NOTE]
> This is a stacked PR. (main <=
Comfy-Org#7478 <=
Comfy-Org#7479)
Enferlain pushed a commit to Enferlain/ComfyUI_frontend that referenced this pull request Dec 18, 2025
## Summary

Show a warning toast when leaving Server Config with pending changes,
reminding users they must restart to apply changes.

## Changes

- **What**: Add a `onBeforeUnmount` toast in `ServerConfigPanel` when
`modifiedConfigs` is non-empty and the user didn’t click Restart; add
i18n strings.

## Review Focus

- Confirm the toast timing/conditions are correct (only fires on leaving
the panel; suppressed when Restart is clicked).

> [!NOTE]
> This is a stacked PR. (main <=
Comfy-Org#7478 <=
Comfy-Org#7479)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7479-feat-server-config-restart-required-toast-2ca6d73d3650811f85f7f0c52c4cf8f0)
by [Unito](https://www.unito.io)
Yourz pushed a commit that referenced this pull request Dec 24, 2025
Adds a Desktop (Electron) Server-Config setting for
`--enable-manager-legacy-ui` so users can opt into ComfyUI-Manager’s
legacy UI.

- Adds `enable-manager-legacy-ui` to `SERVER_CONFIG_ITEMS`
- Adds EN i18n label + tooltip

Note: this PR only adds the setting/flag wiring; it does not change
restart behavior in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7478-feat-server-config-add-legacy-manager-UI-toggle-2ca6d73d365081a79bb2c376506f5346)
by [Unito](https://www.unito.io)


> [!NOTE]
> This is a stacked PR. (main <=
#7478 <=
#7479)
Yourz pushed a commit that referenced this pull request Dec 24, 2025
## Summary

Show a warning toast when leaving Server Config with pending changes,
reminding users they must restart to apply changes.

## Changes

- **What**: Add a `onBeforeUnmount` toast in `ServerConfigPanel` when
`modifiedConfigs` is non-empty and the user didn’t click Restart; add
i18n strings.

## Review Focus

- Confirm the toast timing/conditions are correct (only fires on leaving
the panel; suppressed when Restart is clicked).

> [!NOTE]
> This is a stacked PR. (main <=
#7478 <=
#7479)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7479-feat-server-config-restart-required-toast-2ca6d73d3650811f85f7f0c52c4cf8f0)
by [Unito](https://www.unito.io)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants