Skip to content

feat(view): multi-select items and copy as a Markdown list (0.3.1) - #5

Merged
mabry1985 merged 1 commit into
mainfrom
feat/multiselect-copy-md
Sep 13, 2026
Merged

mabry1985 merged 1 commit into
mainfrom
feat/multiselect-copy-md

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Multi-select + Copy as Markdown

Select rows with the checkboxes (or select-all for the visible rows), then Copy as Markdown or ⌘C. One line per item:

- Reikland Reavers Human Team — Blood Bowl 2016 Split — $57.69
- Loose dice — —

Selection is kept by id across filters and tabs, so a list can span lots. Clipboard falls back to execCommand("copy") and then to a dialog with the text pre-selected when the iframe is denied clipboard access.

Node harness pins the format; 123 tests. Smoke-tested locally with a stubbed clipboard (button and shortcut paths).

🤖 Generated with Claude Code

https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

Josh: "as a user I want to be able to multiselect items and copy them to clipboard as md
list item category price".

- A checkbox column with a select-all for the visible rows; selection survives filter and
  tab changes (kept by id, with the item records cached), so a list can span lots.
- A selection bar with the count, "Copy as Markdown" and "Clear". ⌘C / Ctrl+C copies too
  when rows are selected and focus is not in a field or on a text selection.
- One line per item: `- Name — Category — $price` (category omitted when blank; the price
  is the target, "—" when unpriced; whitespace in names collapsed).
- Clipboard: navigator.clipboard first, execCommand("copy") fallback, and when the frame
  is denied both a dialog with the text pre-selected.
- Tests: the node harness pins the exact list format; presence checks for the controls.
  Smoke-tested in the local harness with a stubbed clipboard (button and shortcut).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

QA panel review — WARN

code-review-structural · head ac542f4d728e · formal

Low-risk change overall. The single confirmed finding is a minor correctness issue: state.itemsById is populated in loadCore() but never pruned, so a server-side delete leaves a stale object that can leak into markdown output via selectedItems(). The verifier confirmed the finding and noted that .filter(Boolean) does not protect against this (the stale value is a truthy object, not undefined). No panel disagreement — the finding was flagged by both correctness and removed-behavior angles and confirmed on verification. No coverage gaps; the structural pass completed and the diff is small enough that one file carries the full risk surface.

Findings

Severity Location Finding Verified
🟡 minor view.py:192 state.itemsById is only ever populated (add/update) and never pruned, so after an item is deleted from the server its stale object remains in the map and is in… confirmed
findings JSON (machine-readable)
[
  {
    "file": "view.py",
    "line": 192,
    "severity": "minor",
    "category": "correctness",
    "claim": "state.itemsById is only ever populated (add/update) and never pruned, so after an item is deleted from the server its stale object remains in the map and is included in the Markdown copy if its ID is still in state.selected (flagged by both correctness and removed-behavior review).",
    "evidence": "for (const it of state.items) state.itemsById[it.id] = it;",
    "verdict": "confirmed",
    "note": "Verified in the full PR diff: itemsById is populated in loadCore() but never pruned (no delete/reset anywhere in the diff). selectedItems() uses .filter(Boolean) which does NOT protect against this \u2014 the stale entry is a truthy object, not undefined. Consequence (stale item in markdown) follows directly."
  }
]

@mabry1985
mabry1985 merged commit aa41a30 into main Sep 13, 2026
2 of 3 checks passed
@mabry1985
mabry1985 deleted the feat/multiselect-copy-md branch September 13, 2026 02:41
mabry1985 added a commit that referenced this pull request Sep 13, 2026
…ot → items (0.4.0) (#6)

* feat: optional game system per item; Markdown copy grouped system → lot → items

Josh: "we also need a 'system' optional field for each item so that we can sort by game
system. so when we copy it is md and each is organized by game system header > lot".

- Store: `items.system` (TEXT, default '') added through the additive-migration path — an
  existing 0.3.x database gains the column on open (tested). Lists order by
  system → lot → category → id; `system=` filter (case-insensitive); search covers it;
  `systems()` lists the values in use.
- CSV alias `system` / `game_system` / `game`; exported. Tool: `inventory_upsert_item(system=)`,
  `inventory_list(system=)`. API: `?system=` on /items, `GET /systems`.
- View: a System column (double-click to edit, with suggestions from the systems in use),
  a System filter, the field in the item dialog. Copy as Markdown now emits
  `## <system>` → `### <lot name>` → `- Name — Category — $price`, blank system as "Other"
  and no-lot as "No lot", both last.
- Vera's WARN on #5 (stale item cache could leak a deleted row into the copy): the copy now
  re-reads the live item list, drops ids that no longer exist (and says so in the toast),
  and the delete action prunes the cache and selection.
- 128 tests (was 123). 0.4.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

* fix(view): reload the grid after a copy skipped rows that no longer exist

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UQ33xXfygwEkzbbJx3YwZ

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.

1 participant