Skip to content

Give the sign simulator and the client icons of their own - #37

Merged
mjaksn merged 2 commits into
mainfrom
tool-icons
Sep 2, 2026
Merged

Give the sign simulator and the client icons of their own#37
mjaksn merged 2 commits into
mainfrom
tool-icons

Conversation

@mjaksn

@mjaksn mjaksn commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Both desktop tools showed Python's icon on the taskbar and Qt's stock one in the title bar. Each now carries a drawing of its own beside its code: the simulator's is the sign itself, amber dots on black showing a message, and the client's is a paper plane on a blue tile. The two are deliberately nothing alike so that they are told apart at a glance.

  • icon.svg beside each tool is the drawing to edit; the icon.ico beside it is rendered from it by scripts/render_icons.py, which packs eight sizes from 16 to 256 pixels itself, since Qt's writer holds one image per file. Its --check runs in the lint job so a drawing cannot change without its rendering.
  • Each app.py sets the icon on the application, so dialogs get it too, and on Windows names its process to the taskbar as readerboard.<identifier>. Without that the taskbar files the window under python.exe, and the icon reaches the title bar only.
  • The simulator's names.py gains the organisation constant the client already had, pinned by the naming test for both tools. tests/test_tool_icons.py checks the icon container without Qt.
  • Both READMEs, AGENTS.md and the changelog describe it, including the taskbar's icon cache, which shows the previous drawing for a minute or so after a relaunch under the same name.

One thing this run of CI settles: the icon check compares a render on the Linux runner against files rendered on Windows, within a small per-channel tolerance. If the runner lands outside it, the fix is to widen the tolerance or commit icons rendered on Linux.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The rendering script does not retain its QGuiApplication, allowing PySide to destroy it before subsequent Qt operations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds distinct icons for both desktop tools, including Windows taskbar integration and rendering validation.

Changes:

  • Adds SVG/ICO artwork and application icon setup.
  • Adds deterministic ICO generation and CI validation.
  • Documents and tests icon and component-name conventions.
File summaries
File Description
tools/signsim/signsim/names.py Adds the organization identifier.
tools/signsim/signsim/icon.svg Adds simulator artwork.
tools/signsim/signsim/icon.ico Adds the rendered simulator icon.
tools/signsim/signsim/app.py Configures application and taskbar icons.
tools/signsim/README.md Documents icon maintenance and caching.
tools/apiclient/apiclient/icon.svg Adds client artwork.
tools/apiclient/apiclient/icon.ico Adds the rendered client icon.
tools/apiclient/apiclient/app.py Configures application and taskbar icons.
tools/apiclient/README.md Documents icon maintenance and caching.
tests/test_tool_icons.py Validates ICO structure and sizes.
tests/test_component_names.py Pins both organization identifiers.
scripts/render_icons.py Renders and checks multi-resolution icons.
.github/workflows/ci.yml Adds icon freshness validation.
CHANGELOG.md Records the new icons.
AGENTS.md Documents the icon workflow.
Review details
  • Files reviewed: 11/15 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/render_icons.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@mjaksn
mjaksn merged commit 4eb3d8c into main Sep 2, 2026
12 checks passed
@mjaksn
mjaksn deleted the tool-icons branch September 2, 2026 04:29
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.

2 participants