Skip to content

Add Qt system tray support - #194

Open
20plays wants to merge 3 commits into
eklonofficial:mainfrom
20plays:review/system-tray-pr
Open

20plays wants to merge 3 commits into
eklonofficial:mainfrom
20plays:review/system-tray-pr

Conversation

@20plays

@20plays 20plays commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds system tray support for the Qt version of Vice.

When a tray is available:

  • Minimize hides Vice to the tray.
  • Closing the window with X hides it to the tray instead of leaving the recorder running with no visible UI.
  • Clicking the tray icon restores the window.
  • The tray menu includes Open Vice and Quit Vice.
  • Quit Vice stops the recorder as well as the GUI.
  • Launching Vice again while it is already running restores the existing window, including on Wayland where wmctrl is not reliable.

If no system tray is available, Vice keeps its existing behavior.

The tray uses Vice's existing icon and is implemented with Qt's QSystemTrayIcon, so it is not KDE-specific.

Tested on KDE Plasma Wayland. The full test suite also passes on Python 3.10, 3.12, and 3.13.

Happy to adjust the implementation if there is a different approach you would prefer.

Copy link
Copy Markdown
Owner

The 11 tray tests pass locally. There is a shutdown detail to address before this goes in, particularly on desktops without a tray host.

WindowTrayController.keep_running() calls win.destroy() directly in that fallback. I reproduced a hang in the existing app with this same call order on pywebview 6.2.1 / Qt: the window closes, then the bridge worker tries to return the API result through evaluate_js after the Qt event loop has stopped. The process and single-instance lock stay alive. A mocked destroy() doesn't catch it. The bridge needs to finish returning its result before the window is destroyed.

Please cover Keep running without a tray host in a real native window, and shutdown failure as well. _shutdown_worker currently closes the GUI even when the daemon shutdown callback raises. I'm also working on removing the old launcher's forced kill after a shutdown timeout, so that part will need reconciling before merging.

@eklonofficial eklonofficial added the status: waiting on author PR review found changes that the contributor needs to make. label Sep 5, 2026
@20plays

20plays commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed reproduction. I have reconciled the branch with the current main branch and routed the no-tray Keep running path through the bridge-safe close helper, so the API response completes before the Qt window is destroyed. Shutdown failures now keep the GUI open and allow retry, and I added a real pywebview 6.2.1/Qt offscreen regression covering process exit and lock release, plus failure-path coverage. The relevant tray/native, reliability, daemon-ownership, runtime, and UI-static suites pass.

@eklonofficial eklonofficial added status: unaddressed Actionable work that has not started. status: waiting on author PR review found changes that the contributor needs to make. and removed status: waiting on author PR review found changes that the contributor needs to make. labels Sep 17, 2026
@eklonofficial

Copy link
Copy Markdown
Owner

This branch now conflicts with current main after the 2.11.x and 2.12.0 fixes. Please rebase or merge current main into the branch and push the result before review. I will rerun the tray and UI checks then.

@eklonofficial eklonofficial removed the status: unaddressed Actionable work that has not started. label Sep 19, 2026
@20plays
20plays force-pushed the review/system-tray-pr branch from 8cdedf7 to bedf86b Compare September 19, 2026 19:56
@20plays

20plays commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Should be good now. cheers

@eklonofficial

Copy link
Copy Markdown
Owner

Thanks, the rebase landed. Two things and then I think this is ready.

The branch conflicts again, and only on the built bundle, which my own UI work on main rewrote. Nothing in ui-src conflicts. Merging main and rebuilding is the whole fix:

git merge main
npm run build
git add vice/ui/scripts/app.js vice/ui/styles/app.css && git commit

The other thing is your opt-in native test. It fails here, and not because of your code: with QT_QPA_PLATFORM=offscreen and DISPLAY and DBUS stripped, QSystemTrayIcon.isSystemTrayAvailable() still returns true on this machine, so keep_running hits your own raise RuntimeError("the native test requires a desktop without a tray host"), the window never closes and the parent times out at 20 seconds. The default suite skips the file so CI stays green, but anyone who sets VICE_RUN_NATIVE_QT_TESTS=1 sees it fail. Please force the no-tray condition in the child rather than inferring it from the environment, so the test proves the fallback on any machine.

What I checked in the meantime: merged your branch into current main locally, rebuilt the bundle, and the suite is clean at 683 tests. I read the close, quit and no-tray paths: the X button only hides when a tray host is really there, Quit keeps the window open when shutdown fails, and Minimize keeps the existing behaviour without a tray, which is what I wanted after #198.

@eklonofficial

Copy link
Copy Markdown
Owner

Thanks for fixing the native test. It passes here now with VICE_RUN_NATIVE_QT_TESTS=1, on the same machine where it timed out before, so forcing the no-tray condition did the job.

I merged your branch into current main locally, rebuilt the bundle and ran everything: the suite is clean.

It conflicts again, and again only on the built bundle, because 2.13.1 landed after your merge. Same fix as last time:

git merge main
npm run build
git add vice/ui/scripts/app.js vice/ui/styles/app.css && git commit

The fixes I have queued for the next release do not touch the UI, so if you do it now it should stay clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting on author PR review found changes that the contributor needs to make.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants