Review follow-ups: PR #615 — feat(labels): default web print to the connected client when no Niimbot #865

New Issue

2026-09-27T02:30:30Z

gambit-admin commented

2026-09-27 02:30:30 +00:00

Non-blocking findings from the auto code review of PR #615 (feat(labels): default web print to the connected client when no Niimbot) — captured for batch cleanup rather than blocking the PR.
Commit adf5d5c · https://github.com/Gambit-Inc/gambit/pull/615

SHOULD-FIX:

frontend/app/hq/inventory/page.js:530 — usePrintTarget(apiFetch) sits at the top of ActionModal, so every action (move, reprice, archive and so on) fires a /print/status request on open, not just "Print labels". ItemDrawer (line 792) does the same on every drawer open, and PrintPip adds a third request. Only the labels action needs it. Move the hook into a labels-only child component, or accept an enabled flag.
frontend/app/hq/labels/usePrintTarget.js:25 — Client liveness is read once on mount and never refreshed. The backend window is 90s (backend/app/routes/printing.py:37). A drawer or modal left open, or a shell that stays mounted for hours, keeps showing "client" after the Pi goes offline, and prints get queued to a dead client. The reverse also happens: a client that comes online later stays "none". The old PrintPip had the same one-shot fetch, but now the print routing depends on it, not just a status badge. At minimum, re-check when the modal or drawer opens or before a print action. Better, poll on an interval.
frontend/app/hq/labels/usePrintTarget.js:38 — Any transient /print/status failure (network blip, 5xx) resolves to "none". That silently routes an actual Pi user to the USB Niimbot connect popup, which is the behaviour this PR removes. Consider treating errors as a distinct state, or keep the "Queue to print client" fallback button visible.
frontend/app/hq/inventory/page.js:701 — Copy mismatch when printTarget === "none". The text says "Connect a Niimbot below", but the only button below is "Queue to print client instead", and no Niimbot connect control exists there. The primary apply button does prompt for connection, but the text points users at the wrong action. Also, "Queue to print client instead" is offered when the status check said no client is online, so it will likely fail.
frontend/app/hq/inventory/page.js:856 — ItemDrawer.printLabel duplicates the applyLabels body: the same raster augmentation, the same queue POST and the same hard-coded defaults (|| "A1", the 100-cent floor). The two can drift, as the #414 comment shows already happened once. Extract a shared queueLabel(item) helper.

NITS:

frontend/app/hq/inventory/page.js:952 — The button label is a nested four-way ternary inline in JSX. It is hard to read and easy to get wrong, so compute it in a variable above the return.

Filed from a Claude Code session

Non-blocking findings from the auto code review of **PR #615** (feat(labels): default web print to the connected client when no Niimbot) — captured for batch cleanup rather than blocking the PR. Commit `adf5d5c` · https://github.com/Gambit-Inc/gambit/pull/615 SHOULD-FIX: - `frontend/app/hq/inventory/page.js:530` — `usePrintTarget(apiFetch)` sits at the top of `ActionModal`, so every action (move, reprice, archive and so on) fires a `/print/status` request on open, not just "Print labels". `ItemDrawer` (line 792) does the same on every drawer open, and `PrintPip` adds a third request. Only the labels action needs it. Move the hook into a labels-only child component, or accept an `enabled` flag. - `frontend/app/hq/labels/usePrintTarget.js:25` — Client liveness is read once on mount and never refreshed. The backend window is 90s (`backend/app/routes/printing.py:37`). A drawer or modal left open, or a shell that stays mounted for hours, keeps showing "client" after the Pi goes offline, and prints get queued to a dead client. The reverse also happens: a client that comes online later stays "none". The old `PrintPip` had the same one-shot fetch, but now the print routing depends on it, not just a status badge. At minimum, re-check when the modal or drawer opens or before a print action. Better, poll on an interval. - `frontend/app/hq/labels/usePrintTarget.js:38` — Any transient `/print/status` failure (network blip, 5xx) resolves to "none". That silently routes an actual Pi user to the USB Niimbot connect popup, which is the behaviour this PR removes. Consider treating errors as a distinct state, or keep the "Queue to print client" fallback button visible. - `frontend/app/hq/inventory/page.js:701` — Copy mismatch when `printTarget === "none"`. The text says "Connect a Niimbot below", but the only button below is "Queue to print client instead", and no Niimbot connect control exists there. The primary apply button does prompt for connection, but the text points users at the wrong action. Also, "Queue to print client instead" is offered when the status check said no client is online, so it will likely fail. - `frontend/app/hq/inventory/page.js:856` — `ItemDrawer.printLabel` duplicates the `applyLabels` body: the same raster augmentation, the same queue POST and the same hard-coded defaults (`|| "A1"`, the 100-cent floor). The two can drift, as the #414 comment shows already happened once. Extract a shared `queueLabel(item)` helper. NITS: - `frontend/app/hq/inventory/page.js:952` — The button label is a nested four-way ternary inline in JSX. It is hard to read and easy to get wrong, so compute it in a variable above the return. --- Filed from a Claude Code session

gambit-admin added the enhancement label 2026-09-27 02:30:30 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gambit/gambit#865