chat: add close to tray

bgallois commented 1 month ago

As suggested in issue #2844, added the option to close to the tray:

[x] An option to activate or deactivate this behavior (default is no tray, as before).
[x] Minimize to the tray.
[x] Restore or quit from the tray icon.

manyoso commented 1 month ago

Lemme know when this is ready to review by taking it out of draft and awesome first contribution!

bgallois commented 1 month ago

The conditionals are a little bit trickier than I anticipated with the recursive close form on onSaveChatsFinished, but the behavior is as follows:

Same as before without the tray activated.

With the tray:

Save chats when the window is closed to the tray (to avoid losing chats if the computer is shut down while the app is still in the tray, the window is invisible, or the system kills the app).
Restore from the tray to make the window visible again.
Quitting from the tray triggers the usual close sequence by saving and closing the app.

Functionality can be built upon this, such as displaying a tray alert when generating something significant while the app is in the tray, etc...

manyoso commented 1 month ago

I see this error when I attempt to build your branch:

-- Found QtIFW 4.5.2 version CMake Error at /home/atreat/Qt/6.7.2/gcc_64/lib/cmake/Qt6/QtPublicWalkLibsHelpers.cmake:259 (message): The Widgets target is mentioned as a dependency for SingleApplication, but not declared.

bgallois commented 1 month ago

What command and flags are you using to compile? I'm unable to reproduce the issue. Have you started from a clean slate? I noticed that after changing the SingleApplication dependency, it was necessary to remove any old build artifacts completely.

The Widget target should normally be handled in the back by SingleApplication.

manyoso commented 1 month ago

See: https://github.com/bgallois/gpt4all/pull/1

bgallois commented 1 month ago

Okay, the simplification is not working as expected, or at least it doesn't implement the same set of features:

Feature Changes:

Right-clicking on the tray icon maximizes the window, making the "Restore" button useless. The "Restore" button should be removed if this behavior is preferred.
The main simplification was removing the chat saving when minimized, so during a computer shutdown, the chat will not be saved if the user doesn't manually close the program. This seems risky to me because users usually tend to forget the minimized program.

Bugs:

Quitting the program is now impossible in tray mode. Clicking "Quit" in the tray will hide the tray icon but doesn't close the software, and the window also needs to be closed so the program finally closes.
Adding "widget" to the CMakeLists.txt fixes the problem that I can reproduce, but it seems illogical because previously "gui" was required but wasn't explicitly added to CMakeLists.txt, as it was handled by the SingleApplication dependency. "widget" should follow the same principle and be added automatically (that what appends on my system).

You are right that using raise() and requestActivate() is better for restoring the window, and that left-clicking the icon should restore the window.

manyoso commented 1 month ago

Okay, the simplification is not working as expected, or at least it doesn't implement the same set of features:

Feature Changes:

Right-clicking on the tray icon maximizes the window, making the "Restore" button useless. The "Restore" button should be removed if this behavior is preferred.

I'm testing on linux. Right clicking only activates the menu for me for some reason. I will fix this pre-emptively by looking at the 'reason' arg in onActivated. The 'restore' can menu item can stay even if it is redundant as you can left click or middle click to restore.

The main simplification was removing the chat saving when minimized, so during a computer shutdown, the chat will not be saved if the user doesn't manually close the program. This seems risky to me because users usually tend to forget the minimized program.

I'll fix this.

Bugs:

Quitting the program is now impossible in tray mode. Clicking "Quit" in the tray will hide the tray icon but doesn't close the software, and the window also needs to be closed so the program finally closes.

Clicking 'Quit' does quit the program for me. You're testing on mac or windows? I'm on linux, so perhaps we have different behavior?

Adding "widget" to the CMakeLists.txt fixes the problem that I can reproduce, but it seems illogical because previously "gui" was required but wasn't explicitly added to CMakeLists.txt, as it was handled by the SingleApplication dependency. "widget" should follow the same principle and be added automatically (that what appends on my system).

Without this I was getting build error.

bgallois commented 1 month ago

I'm also testing on Linux (and after a system restart and a clean build the program is now quitting correctly but not restored by left-click, so something is not stable). The build error is likely masking another error because it isn't logical and reproducible, and maybe it should be investigated more closely.

Feel free to push directly to this branch to complete and close this PR with the features as you see fit.

manyoso commented 1 month ago

I'm also testing on Linux (and after a system restart and a clean build the program is now quitting correctly but not restored by left-click, so something is not stable). The build error is likely masking another error because it isn't logical and reproducible, and maybe it should be investigated more closely.

Feel free to push directly to this branch to complete and close this PR with the features as you see fit.

Unfortunately, I can't push directly to this PR as it is open on your fork I think. The widgets dependency I'll look into as I guess your thesis is that it should be handled by the SingleApplication cmake file?

More concerning is the weird behavior on left click not working for you and right click not working for me... seems unstable. This is in "labs" and is it possible that the underlying Qt qml class is just buggy?? If so, we can't add it yet...

bgallois commented 1 month ago

I allowed "Allow edits by maintainers" so you should be able to push on it.

The widgets dependency I'll look into as I guess your thesis is that it should be handled by the SingleApplication cmake file?

Yes, like it was the case before with the Gui dependency necessary for the QGuiApplication.

More concerning is the weird behavior on left click not working for you and right click not working for me... seems unstable. This is in "labs" and is it possible that the underlying Qt qml class is just buggy?? If so, we can't add it yet...

Maybe it is better to implement it with the reason as you suggested, to remove any "default" behavior that can depend on the platforms and systems.

manyoso commented 1 month ago

@bgallois can you test the latest version I just pushed and review to see if you're happy with it?

bgallois commented 1 month ago

It is working perfectly for me.

manyoso commented 1 month ago

There are some inconsistencies and problems across the three platforms i'm testing on now: linux, windows, and macos. Trying to resolve them all... but still no dice.

nomic-ai / gpt4all