Hide ComfyUI instead of quitting when window is closed

[crowecawcaw]

This change makes it more intuitive to keep ComfyUI running in the background. Previously, if the ComfyUI window was closed, the app quit. With this change, closing the window keeps ComfyUI running, and the window can be restored from the "Show Comfy Window" taskbar option. On a Mac, when the window is closed, the dock icon is also hidden.

Tested on Mac and Windows.

┆Issue is synchronized with this Notion page by Unito

[webfiltered]

Whilst this is expected behaviour for some apps on macOS, on Windows many (most?) apps with this functionality have it off by default.

If we were to offer this as a feature, it would need to be optional (and I believe opt-in). Tagging this for design review!

[crowecawcaw]

Updated to make the feature opt-in based on a new setting. I'm not familiar with the release ordering for the various packages, but after this is merged, I can add the setting to the ComfyUI settings panel. I tested the change by manually editing the settings file on Mac and Windows.

[webfiltered]

Some design questions, test issues, and a few nits.

[webfiltered]

When the setting is updated in frontend the state should be sent to the desktop process and kept synchronised (e.g. AppState). IPC is mostly handled in preload.ts, constants.ts.

Dynamically calling settings every time performs a round-trip IPC call to a single-threaded browser process.

[crowecawcaw]

Adding a new IPC handler with logic on both sides seems a bit over-engineered to me. Even if we read the setting every time, seems like it'd be fast enough for an infrequent even. Other code seems to be using a similar approach.

If we did want to IPC in here, it might make sense to make a generic settings syncing mechanism that would transfer the whole set of settings from the frontend whenever it's updated, then the desktop app can just read the local settings copy. That's a bit out of scope of this PR though.

Let me know your thoughts. If you still prefer IPC syncing, I can add it.

[webfiltered]

Hey, thanks for taking the time to respond. I've been out of action for a bit.

For this one, it's more about how the app should work, and preventing weird error states/lockups by just using proper separation of concerns. Using code running in a browser process (which isn't always available) as the runtime memory storage for the parent process' state is just too circular and brittle.

p.s. If you can point me at the other code, I'll add it to the issues list.

[crowecawcaw]

Looking closer at this, useSettings().get('key') looks like it just reads a key from an in-memory object: https://github.com/Comfy-Org/desktop/blob/main/src/config/comfySettings.ts#L153

So to make this work correctly, I'm thinking I'll add a new IPC handler for reloadComfySettings. The frontend will call after updating a setting, and in response the backend code will reread the settings file from disk. This mechanism will work more broadly with other settings that the frontend updates which need to be synced to the server.

Does this sound right to you? It would not touch AppState and would continue to use useSettings(). I might have missed a detail of the implementation or misunderstood what you were pointing out.