feat(client): replace the charges table with a per-type charge record list - #4231
Open
gilgardosh wants to merge 2 commits into
Open
feat(client): replace the charges table with a per-type charge record list#4231gilgardosh wants to merge 2 commits into
gilgardosh wants to merge 2 commits into
Conversation
gilgardosh
temporarily deployed
to
accounter-fullstack
August 17, 2026 16:03 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
gilgardosh
temporarily deployed
to
accounter-fullstack
August 17, 2026 16:03 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@accounter/client |
0.1.0-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/green-invoice-graphql |
0.8.7-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/hashavshevet-mesh |
0.2.13-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/israeli-vat-scraper |
0.1.13-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/modern-poalim-scraper |
0.11.0-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/payper-mesh |
0.2.13-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/scraper-app |
0.0.3-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/server |
0.2.0-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/shaam-uniform-format-generator |
0.2.7-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
@accounter/shaam6111-generator |
0.1.9-alpha-20260824115950-8e76ec21f4e292cf778d572d2164c4227d15ab7e |
npm ↗︎ unpkg ↗︎ |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the client charges list UI from a traditional table layout into a per-charge “record list” whose visible fields are driven by a declarative per-type spec matrix, while keeping TanStack Table as the headless engine (selection, expansion, sorting).
Changes:
- Replace table row/cell rendering with a record-based list (
ChargeRecord+ regions) and a per-type field visibility matrix (charge-fields.ts). - Move list-level controls into a dedicated toolbar (selection count, batch actions, density toggle, CSV export, and server/client sort menu).
- Add new indicators/suggestion affordances and fix several behavioral issues (pagination truncation, selection/export leakage, batch refresh semantics, drag/drop interactivity, accessibility labeling).
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client/src/components/screens/charges/missing-info-charges.tsx | Binds server-side sort into screen-owned filter and passes sort binding to ChargesTable. |
| packages/client/src/components/screens/charges/charge.tsx | Hides list toolbar on single-charge screen. |
| packages/client/src/components/screens/charges/all-charges.tsx | Adds server-side sort binding passed to ChargesTable. |
| packages/client/src/components/common/inputs/drag-file.tsx | Restores pointer events inside Mantine Dropzone content so record content remains interactive. |
| packages/client/src/components/charges/utils.ts | Removes legacy per-type “shouldHaveX” helpers (superseded by spec matrix). |
| packages/client/src/components/charges/use-charge-density.ts | Adds persisted per-browser density toggle with same-tab synchronization. |
| packages/client/src/components/charges/columns.tsx | Removes legacy presentational TanStack column definitions (table layout). |
| packages/client/src/components/charges/columns.ts | Adds accessor-only column schema used solely for sorting capabilities. |
| packages/client/src/components/charges/charges-toolbar.tsx | Adds list toolbar for selection/batch actions/sort/density/export. |
| packages/client/src/components/charges/charges-table.tsx | Converts table rendering into <ul> record list, adds refetch registry, fixes pagination truncation, export selection isolation, and integrates toolbar. |
| packages/client/src/components/charges/charges-sort-menu.tsx | Adds shared sort dropdown for server-scope and page-scope sorting. |
| packages/client/src/components/charges/charges-row.tsx | Removes legacy <tr> row + expansion-row rendering and row-model mutation. |
| packages/client/src/components/charges/charges-filters.tsx | Removes sort controls from filters form and preserves toolbar-driven sort on submit/clear. |
| packages/client/src/components/charges/charges-batch-actions-menu.tsx | Switches refresh-selected behavior to refetch-registry callback. |
| packages/client/src/components/charges/charge-suggestion-field.tsx | Adds unified suggestion UI/behavior for description and tags (one-click accept + apply-to-similar). |
| packages/client/src/components/charges/charge-record.tsx | Implements memoized charge record list item, DnD wrapper, and inline expansion panel. |
| packages/client/src/components/charges/charge-record.stories.tsx | Adds Storybook coverage for all types, density, missing-info, and edge cases. |
| packages/client/src/components/charges/charge-record-regions.tsx | Implements the six fixed-position regions composing each record. |
| packages/client/src/components/charges/charge-indicators.tsx | Adds indicator primitives (needs badge, dots, chips, VAT/amount/ledger state). |
| packages/client/src/components/charges/charge-indicators.stories.tsx | Adds Storybook gallery for indicator states and suggestion fields. |
| packages/client/src/components/charges/charge-fields.ts | Adds per-type field visibility matrix and helpers (isFieldVisible, relevantMissingInfo, etc.). |
| packages/client/src/components/charges/charge-fields.stories.tsx | Adds Storybook “Spec Matrix” story for validating matrix transcription. |
| packages/client/src/components/charges/charge-dates.ts | Extracts/renames date aggregation logic from old cell into reusable helpers. |
| packages/client/src/components/charges/cells/vat.tsx | Removes legacy VAT cell implementation. |
| packages/client/src/components/charges/cells/type.tsx | Removes legacy charge type cell implementation. |
| packages/client/src/components/charges/cells/tax-category.tsx | Removes legacy tax category cell implementation. |
| packages/client/src/components/charges/cells/tags.tsx | Removes legacy tags cell implementation. |
| packages/client/src/components/charges/cells/more-info.tsx | Removes legacy “More Info” cell (counts + DnD drop target). |
| packages/client/src/components/charges/cells/index.ts | Removes legacy cell exports barrel. |
| packages/client/src/components/charges/cells/description.tsx | Removes legacy description cell implementation. |
| packages/client/src/components/charges/cells/counterparty.tsx | Removes legacy counterparty cell implementation. |
| packages/client/src/components/charges/cells/business-trip.tsx | Removes legacy business trip cell implementation. |
| packages/client/src/components/charges/cells/amount.tsx | Removes legacy amount cell implementation. |
| packages/client/src/components/charges/tests/pagination.test.tsx | Adds regression test for pagination truncation behavior. |
| packages/client/src/components/charges/tests/columns.test.tsx | Adds regression tests for date/amount sorting correctness. |
| packages/client/src/components/charges/tests/charges-toolbar.test.tsx | Adds tests for toolbar behavior (selection count, sort bindings, density toggle, etc.). |
| packages/client/src/components/charges/tests/charge-indicators.test.tsx | Adds tests for indicator semantics and accessibility labeling. |
| packages/client/src/components/charges/tests/charge-fields.test.ts | Adds tests for matrix invariants and missing-info filtering behavior. |
| packages/client/src/components/charges-ledger-validation.tsx | Adds server-side sort binding passed to ChargesTable. |
| packages/client/src/components/business/charges-section.tsx | Removes redundant border wrapper around ChargesTable (list now draws its own). |
| .changeset/charges-record-list.md | Documents the UI refactor and enumerates user-visible changes/bug fixes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+393
to
+397
| // Intersected with this table's own charges: the VAT report shares one selection map across three | ||
| // tables, so an unfiltered selection would leak the other tables' charges into this export. | ||
| const selectedIds = Object.keys(rowSelection).filter( | ||
| id => rowSelection[id] && chargeIds.includes(id), | ||
| ); |
Comment on lines
+75
to
+80
| <Checkbox | ||
| checked={isSelected} | ||
| onCheckedChange={value => onSelectedChange(!!value)} | ||
| // 100 identical "Select row" labels are useless to a screen reader; name the charge. | ||
| aria-label={`Select charge ${row.description ?? row.id}`} | ||
| /> |
Comment on lines
+31
to
+39
| it('gives every type a full-width row', () => { | ||
| // Rows are positional against CHARGE_FIELDS; a short row would make later fields read as hidden. | ||
| for (const type of ALL_TYPES) { | ||
| const decided = CHARGE_FIELDS.filter( | ||
| field => isFieldVisible(type, field) || !isFieldVisible(type, field), | ||
| ); | ||
| expect(decided).toHaveLength(CHARGE_FIELDS.length); | ||
| } | ||
| }); |
Comment on lines
+58
to
+69
| type Visibility = 0 | 1 | 2; | ||
|
|
||
| /** | ||
| * The spec matrix. One row per charge type, positional against {@link CHARGE_FIELDS}, mirroring the | ||
| * spec spreadsheet 1:1 so a cell can be diffed against it by eye. | ||
| * | ||
| * Note how little of this a table could express: `vat` and `businessTrip` each apply to exactly one | ||
| * of eleven types, which is why the record composes fields per type rather than sharing columns. | ||
| */ | ||
| // prettier-ignore | ||
| const MATRIX: Record<ChargeType, readonly Visibility[]> = { | ||
| // ty md dr am vat cp de tg tax bt tx doc me ldg |
Comment on lines
+74
to
+78
| /** | ||
| * Client-side sortable columns, labelled. Keyed by the explicit `id`s in `columns.tsx` — which is why | ||
| * those ids are declared rather than derived (tanstack would otherwise name them | ||
| * `counterparty_counterparty_name`). | ||
| */ |
gilgardosh
force-pushed
the
feat/charges-record-list
branch
from
August 23, 2026 10:47
98b144d to
ac8b67a
Compare
gilgardosh
temporarily deployed
to
accounter-fullstack
August 23, 2026 10:47 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
gilgardosh
temporarily deployed
to
accounter-fullstack
August 23, 2026 10:47 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Collaborator
Author
|
Rebased onto #4239 — charge row refetch threading and modal close logic
#4209 — drop the charge row when deleting its last document empties it
Its regression test
73 tests pass in the charges suite, 3647 repo-wide, typecheck and build clean. The one repo-wide failure is the pre-existing |
… list Charges are a union of 11 types with genuinely different attributes, so a shared column set never fitted them: of 14 display attributes, VAT and business trip each apply to exactly one type, leaving those columns blank on ten rows out of eleven. Rows also measured 166px. Each charge now renders as a record of six regions at fixed horizontal positions. Region placement is identical on every record, preserving the vertical scanning a table gave; which fields appear inside a region comes from a declarative matrix in charge-fields.ts, so a record's shape is a pure function of its __typename. Records measure ~85px, or 50px compact. TanStack Table stays as the headless engine, so sorting, selection (keyed by charge id) and expansion are unchanged and every call site keeps the same props. columns.tsx becomes an accessor-only sort schema; the presentational cells and utils.ts (whose per-type rules had drifted from the server's) are removed. Bug fixes found along the way: - The Date column never sorted: its accessor returned the whole date object, so the automatic sort compared "[object Object]" with itself and returned the same answer for every pair, in both directions. - Lists silently truncated at 100 rows, hiding everything past row 100 on the screens that fetch without a limit. - Nothing inside a row was clickable when wrapped for drag-and-drop, because Mantine's dropzone disables pointer events on its content. - The header row rendered one more cell than the body. - CSV export could include charges selected in a sibling table. - Batch refresh did nothing for selected but unrendered charges. - Validation state was colour-only with no accessible name, and the expand control was an unlabelled chevron. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ports an imported design mockup onto the record list. The mockup turned out to be a refinement of the design already in this PR rather than a replacement — same six regions, same order, same 1/2/4/2/2/1 spans, same suggestion and needs-badge vocabulary — so this takes its visual deltas and leaves the working structure alone. Its per-type `show` flags were an independent second transcription of the spec matrix and agreed with `charge-fields.ts` on all 143 cells, so the matrix is unchanged. Per-type colour chip. `CHARGE_TYPE_COLOR` gives each of the eleven types its own hue, rendered by `ChargeTypeBadge` as a tinted ringed icon chip beside the type name; colour is what lets you pick every Salary charge out of a hundred rows without reading. Three hues were reassigned because they collided with the status vocabulary on the same row — amber is "needs attention", emerald "accept / positive", red "error / negative" — so Salary moved amber → purple, Monthly VAT emerald → sky, and Dividend rose → pink. A test asserts that reservation. The hue class lists are complete literals, not interpolated from the hue name, because Tailwind only sees literals in source. Selection and drop feedback. Selected records gain a 2px left accent rail on top of their tint. `DragFile` names its dropzone group so the record styles itself off Mantine's `data-accept` and lights the whole row on drag; before, the only sign a drag had registered was the cursor. The drop ring uses `primary` rather than the `ring` token because `--color-accent` and `--color-muted` hold the same value, leaving the drop tint indistinguishable from the selected background, and a grey ring at 40% did not separate them either. Verified that the record's checkbox, approval control, actions menu and expand button all still hit-test as clickable through the dropzone. One surface. Toolbar and list now share a single bordered card rather than the toolbar floating above a separately bordered list. The expansion panel is indented under its record behind a rail. Confirmed the card's `overflow-hidden` does not clip the panel's nested horizontal scroller: with a forced 4000px child the scroller reports scrollWidth 4000 against clientWidth 1200, scrolls, and the page gains no horizontal scrollbar. Smaller fixes from the same pass. The CSV export gains a visible label, having had no accessible name at all — a tooltip is a description, not a name. Count chips gain hover hints. "Delete Charge" is styled destructive, having been the only irreversible item in the menu at the same weight as "Copy Charge Link" above it. The accountant-status control gains a section label, per-option dot and current-value checkmark, an `aria-label` on its trigger, a note that Pending is a downgrade rather than a step toward approval, and the `dark:` variants it had none of; the VAT and business-trip reports share it and inherit all of that. Its options map keeps its existing shape deliberately — the filter in `charges-filters.tsx` reads it with `Object.values()` and spreads the leftover fields onto a `Button`, so a new key would land on the DOM as an unknown attribute. Charges-surface neutrals move onto the colour tokens; status accents and type hues stay literal, since no token expresses them. The "absent value" placeholder, previously defined identically in two files, is now shared. Not adopted from the mockup, because the existing code is better: `role="status"` on the needs badge (a hundred rows would mean a hundred live regions announcing on every refetch), and a count chip whose state never reaches its accessible name. Its compact row also dropped the approval control and the date and reflowed the spans — approval is the primary triage action and the date the second-most- scanned field, and stable spans mean toggling density does not reflow the columns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gilgardosh
force-pushed
the
feat/charges-record-list
branch
from
August 24, 2026 11:58
ac8b67a to
f9c5bd1
Compare
gilgardosh
temporarily deployed
to
accounter-fullstack
August 24, 2026 11:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
gilgardosh
temporarily deployed
to
accounter-fullstack
August 24, 2026 11:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Collaborator
Author
Design refinement adopted (2nd commit)The imported mockup at The matrix is now independently validatedThe mockup's config file carried its own per-type Adopted
Three judgement calls worth your eye1. Three hues reassigned. Amber means "needs attention", emerald "accept / positive", red "error / negative" — and all three appear on the same row as the type chip. The ref palette put Salary on amber, Monthly VAT on emerald, Dividend on rose, which would have made a Salary chip sit inches from an amber needs badge and stopped amber meaning attention. Salary → purple, Monthly VAT → sky, Dividend → pink; the other eight are untouched. 2. The drop ring is 3. Kept shipped icons. The ref pairs Not adopted — the existing code is better
I also skipped its loading skeleton: VerificationTypecheck 0 errors across the whole client, lint 0 errors, prettier clean, 3666 tests pass (the one failure is the pre-existing Driven live against the mock server, GraphQL confirmed hitting
One note on the screenshots: a 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Charges are a union of 11 types with genuinely different attributes, so a shared column set never fitted them. Working from the attached per-type field spec: of 14 display attributes, VAT and business trip each apply to exactly one type, which left those columns blank on ten rows out of eleven. Counterparty applies to four, tax category to five.
Rows also measured 166px (measured, not estimated) —
ListCapsulerendered tags and each metadata count as its own bordered box, while the select and actions cells stacked their controls vertically and set the row height regardless of content.What changed
Each charge renders as a record of six regions at fixed horizontal positions — manage, identity, meaning, health, money, actions. Region placement is identical on every record, which is what preserves the vertical scanning a table gave you. Which fields appear inside a region comes from a declarative matrix in
charge-fields.ts, so a record's shape is a pure function of its__typename.Records measure ~85px, or 50px with the new density toggle — down from 166px.
TanStack Table stays as the headless engine: sorting, selection (keyed by charge id), expansion and the batch-actions menu are unchanged, and all 8 call sites keep the same props.
columns.tsxbecomes an accessor-only sort schema; the presentational cells andutils.tsare removed (its per-type rules had already drifted from the server'svalidate.helper.ts).User-visible
sortBy, so it orders every matching charge rather than only the loaded page. Select-all, batch actions and CSV export moved there too, and the toolbar announces the selection count.Bugs fixed
"[object Object]"with itself and returned the same answer for every pair — in both directions.charges-ledger-validation(streams without a limit) and the unbounded VAT report sections could never show anything past row 100. LeavingpageSizeunset would have been worse — TanStack defaults to 10.Verification
aria-expanded/aria-controls, panel inside the same<li>), selection, and the sort menu firing exactly one refetch per change with the right variables and a page reset.Three Storybook stories are the review surface:
Reviewer notes
charge-fields.tsand the tests will say which invariant moved.chargesByIDsfabricates ids rather than echoing the requested ones: expand-all's panel contents (the single batched request is confirmed; only its id lookup misses) and the business detail page, whose route loader fails for the same reason. Both want one pass against a real backend.🤖 Generated with Claude Code