Skip to content

Show bill/subscription category as text outside edit mode - #11

Merged
brianramseyau merged 1 commit into
mainfrom
feature/remove-redundant-category-selects
Aug 18, 2026
Merged

brianramseyau merged 1 commit into
mainfrom
feature/remove-redundant-category-selects

Conversation

@brianramseyau

Copy link
Copy Markdown
Owner

What

Bills and Subscriptions pages rendered an always-editable category select in their non-editing table rows, auto-saving a category change on selection without entering edit mode. These rows now show the category name as plain text, matching the Expenses page which already displays category as text outside edit mode.

The category select remains in:

  • the edit row (where it still auto-saves, same as the Expenses edit row)
  • the add-bill / add-subscription form

Why

Category assignment should be an edit-screen action rather than a permanent inline control on every row.

Tests

  • Rewrote the two view-mode category tests on each page to assert the category name is shown with no select outside edit mode, and that the select appears in edit mode and calls the update API with the chosen categoryId.
  • pnpm test (web, 751 tests), pnpm lint, and pnpm typecheck all pass.

Bills and Subscriptions displayed an always-editable category select in
their non-editing rows, auto-saving on change without entering edit mode.
Show the category name as plain text in those rows instead, keeping the
select in the edit row and add form - matching the Expenses page pattern.
@brianramseyau
brianramseyau merged commit 0751bc4 into main Aug 18, 2026
4 checks passed
@brianramseyau
brianramseyau deleted the feature/remove-redundant-category-selects branch August 18, 2026 11:23
brianramseyau added a commit that referenced this pull request Sep 14, 2026
Kilo is active on this repo (confirmed on PR #14) - it was just paused
when Dev was previously stopped. My earlier pass had concluded (from
checking PR #11's reviews/comments and repo files) that no review bot was
configured here and rewrote the workflow accordingly; that was wrong.
Restore the "green check isn't proof there's nothing to fix - read every
comment, reply and resolve each thread individually" steps in AGENTS.md
and PLAN_01_OVERVIEW.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
brianramseyau added a commit that referenced this pull request Sep 14, 2026
…(Phase 0) (#14)

* Fix clipped ActionMenu/HelpTooltip popovers; kick off UI/UX overhaul plan (Phase 0)

The row-action menu (⋮) rendered inside each table's overflow-x-auto card
and always opened downward, so a row near the bottom of the viewport had
its menu clipped or entirely invisible - the last row on Expenses always
did this. HelpTooltip had the same hand-rolled-positioning shape.

Rebuild both on bits-ui: ActionMenu on DropdownMenu, HelpTooltip on
Popover. Both are now portalled and collision-aware, so they flip upward
near the viewport edge instead of clipping. Their public props are
unchanged except the custom `trigger` snippet, which now receives props to
spread rather than an (open, toggle) pair - updated its one caller
(Income's "Add" button).

This is Phase 0 of a broader UI/UX overhaul, planned out in
foundational/PLAN_01_OVERVIEW.md and DESIGN.md ("Polymer" - see that doc
for the concept, palette and signature element). Phase 0 also:
- installs the shadcn-svelte + bits-ui foundation (dropdown-menu, popover,
  button only for now), with the new components' tokens mapped to the
  app's *existing* slate/indigo palette so nothing else changes visually
  yet - Phase 1 does the real palette swap
- updates AGENTS.md with the plan/design pointers, a mandatory
  /frontend-design step for UI work, a browser-screenshot recipe, and a
  per-phase checks-and-reviews workflow matched to this repo's actual CI
  (lint/typecheck/test/e2e - no review bot)
- adds e2e/action-menu.spec.ts, a real-browser regression test asserting
  the last row's menu lands fully inside the viewport

jsdom can't lay out bits-ui's floating content (no real layout engine), so
unit tests needed a few adjustments - see ActionMenu.spec.ts's top-of-file
comment and DESIGN.md's decisions log for what and why. A global
src/tests/setup.ts reset (body style + bits-ui's dismissable-layer
registry) will matter again for every future bits-ui overlay.

Verified: pnpm verify and pnpm test:e2e both green; API coverage
unaffected (no API changes). Manually confirmed against the dev server
that the last row's menu on /expenses is now fully visible in both light
and dark mode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Restore Kilo Code Review to the checks-and-reviews workflow

Kilo is active on this repo (confirmed on PR #14) - it was just paused
when Dev was previously stopped. My earlier pass had concluded (from
checking PR #11's reviews/comments and repo files) that no review bot was
configured here and rewrote the workflow accordingly; that was wrong.
Restore the "green check isn't proof there's nothing to fix - read every
comment, reply and resolve each thread individually" steps in AGENTS.md
and PLAN_01_OVERVIEW.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Fix CI: bump @adonisjs/ace to 14.1.1, pin Node to 24.20.0 everywhere

CI's test/e2e jobs were failing with "Invalid command exported from
demo_seed.js file. Invalid URL" - unrelated to this branch's own changes,
since it throws while apps/api's test bootstrap boots the ace Kernel
(testUtils.db().migrate()), which scans every command file in
commands/ regardless of what's being tested.

Root cause (already diagnosed and fixed the same way in EveryList,
commit 468045b): Node 24.20.0's loader changes broke @adonisjs/ace's
command-metadata validator (adonisjs/ace#169) - its jsonschema-based
validator resolves schema $refs against an implicit base URL that
24.20.0's stricter URL parsing rejects. Fixed upstream in ace 14.1.1
(adonisjs/ace#170, which gives the schema a real $id), but
@adonisjs/core@7.3.3 still depends on ^14.1.0, which resolves to the
broken 14.1.0 by default.

- pnpm-workspace.yaml: override @adonisjs/ace to 14.1.1 project-wide,
  same mechanism already used there for a couple of other transitive
  deps. Verified locally on Node 24.18.1: still 640/640 passing - the
  fix is backward compatible, not just forward-compatible with 24.20.0.
- Pin Node to 24.20.0 everywhere, matching EveryList: new .nvmrc, both
  package.json engines fields (>=24.20.0 <25), CI's node-version, and
  the Dockerfile's base image tag. The Docker image tag pin is cosmetic
  consistency rather than a required fix - the built image runs
  precompiled JS, not the .ts loader path this bug is actually in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Loosen commit/push approval: don't re-ask per fix-up during PR iteration

Approval is now needed once per phase (the commit+push that opens the
PR), not again for every fix-up commit responding to a broken CI check
or review/Kilo feedback within that same PR - the owner will say up
front if they want to hold off on a round instead. Merging, and
starting a new piece of work, still need explicit confirmation.

Also drops the "dev server must stay running" rule - the owner will ask
to hold off ahead of time instead of this needing a standing rule.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Address Kilo Code Review feedback (4 findings)

- ActionMenu.svelte / income/+page.svelte: the default trigger's (and
  Income's custom trigger's) literal `class` attribute was overwriting
  `{...props}`'s class after the spread, silently dropping a caller's
  `class` prop even though it's still declared in Props. Merge with
  `cn()` instead of overwriting.
- HelpTooltip.svelte: `role="tooltip"` on the popover content
  contradicted PopoverTrigger's own `aria-haspopup="dialog"` semantics
  (a true ARIA tooltip is non-interactive, hover-triggered description
  text - this panel is a click-to-open, focus-managed popover). Use
  `role="status"` instead, so the explanation is announced the moment
  it appears rather than needing an ARIA role that doesn't match its
  actual behaviour. Updated the one spec that queried by the old role.
- e2e/action-menu.spec.ts: the regression test only checked the first
  menu item's (Edit) bounding box, which could pass in exactly the
  clipping failure mode it exists to catch if a menu opened with just
  enough room for its first item but not its later ones. Check the
  menu container's own bounding box instead - if the container fits,
  every item inside it does too.
- pnpm-workspace.yaml: fixed the override comment's cited
  @adonisjs/core version (7.3.3 -> the actually-resolved 7.3.5).

Verified: pnpm verify and pnpm test:e2e both still green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Close out Phase 0: log the CI fix and Kilo review round

Note the unrelated Node/ace CI fix and the Kilo Code Review round (4
found, 4 fixed, re-review confirmed clean) in the phase doc's Notes
and deviations. Log the role="tooltip" -> role="status" a11y decision
in DESIGN.md's decisions log for future click-to-reveal panels to
follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Fix phase doc wording: engines is a floor, not an exact pin

Kilo caught it: the phase doc said Node was "pinned to 24.20.0
everywhere", but the two package.json engines fields set a
>=24.20.0 <25 range - only .nvmrc, CI and the Dockerfile are an exact
pin. Reworded to say that accurately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <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