Skip to content

fix(settings): clean up Windows UI resources - #47

Merged
laceyp99 merged 2 commits into
mainfrom
t3code/review-open-issues
Sep 9, 2026
Merged

laceyp99 merged 2 commits into
mainfrom
t3code/review-open-issues

Conversation

@laceyp99

@laceyp99 laceyp99 commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • use the canonical AppData directory for generated icons and set Murmur's Windows identity before other UI is created
  • give settings variables explicit window ownership and release native icon handles when each settings window closes
  • restore shared Tk window-class icon handles before releasing settings-window native handles, including the icon-photo failure path
  • remove the related settings and logging maintenance traps, with focused regression coverage

Closes #36.

Validation

  • venv\\Scripts\\ruff.exe format --check .
  • venv\\Scripts\\ruff.exe check .
  • venv\\Scripts\\pytest.exe -q (172 passed)
  • GitHub CI: guardrails and Ubuntu/Windows test jobs passed

Notes

  • No packaging, install, or CI behavior changes.
  • Automated tests cover icon ownership, shared-class restoration, and cleanup; repeated real-window open/close behavior was not manually exercised.
  • Follow-up issue #48 tracks packaging Murmur as a named Windows executable with embedded icon and version metadata. The current PR can merge independently.

@laceyp99
laceyp99 marked this pull request as ready for review September 8, 2026 00:36
@laceyp99

laceyp99 commented Sep 8, 2026

Copy link
Copy Markdown
Owner Author

Validation

  • Full test suite: venv\Scripts\pytest.exe -q172 passed.
  • ruff format --check, ruff check, and git diff --check passed; GitHub Ubuntu and Windows CI passed.
  • On Windows, confirmed %APPDATA%\murmur\murmur.ico was generated and the icon looked correct at native size in the Settings title bar, taskbar, Alt+Tab, and system tray.
  • Closed and reopened the app more than 10 times; the tray and Settings GUI remained responsive.
  • Verified settings persistence: closing without saving discarded the change; saving persisted it after reopen; the original value was restored.

Notes

Task Manager showed Python processes rather than a searchable Murmur identity, which is not a reliable AppUserModelID check for this tray-resident app; taskbar, title bar, Alt+Tab, and tray behavior were correct. The forced native icon-photo failure path is covered by mocked tests only and was not manually induced.

@laceyp99
laceyp99 merged commit c1cd97b into main Sep 9, 2026
6 checks passed
@laceyp99
laceyp99 deleted the t3code/review-open-issues branch September 9, 2026 00:17
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.

Settings GUI polish: app-data path divergence, lazy AppUserModelID, per-open resource leaks, misc nits

1 participant