docs(11-03): complete EditableCell + TagMultiSelect plan
- add 11-03-SUMMARY.md documenting EditableCell/TagMultiSelect delivery and the react-hooks lint-rule fix (Rule 1 deviation) - update STATE.md position (3 of 4 -> 4 of 4), progress 86% -> 89%, performance metrics, and decisions log - log gsd-sdk unavailability and OFFER-07/08 status timing as deferred items for Phase 11
This commit is contained in:
@@ -0,0 +1,133 @@
|
||||
---
|
||||
phase: 11-catalog-database-view-ux-legacy-consolidation
|
||||
plan: 03
|
||||
subsystem: ui
|
||||
tags: [react, tailwind, shadcn, inline-edit, tags, client-components]
|
||||
|
||||
# Dependency graph
|
||||
requires:
|
||||
- phase: 11-catalog-database-view-ux-legacy-consolidation
|
||||
plan: "11-02"
|
||||
provides: "addTagToService / removeTagFromService server actions (entity_type='services', admin-gated, idempotent)"
|
||||
provides:
|
||||
- "EditableCell — generic click-to-edit cell for text/number/textarea/toggle, with Enter/blur save, Escape cancel, required validation, disabled state"
|
||||
- "TagMultiSelect — tag badges with deterministic hash-based pastel colors (no stored color column), inline add/remove dropdown wired to Plan 02's tag actions"
|
||||
- "getTagColorIndex — exported pure name->color-index hash function for reuse/testing"
|
||||
affects: ["11-04"]
|
||||
|
||||
# Tech tracking
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "Click-to-edit cell pattern: display div with cursor-pointer/hover -> on click swaps to Input/Textarea/checkbox with ring-1 ring-primary, Enter/blur commits, Escape reverts to last-saved value"
|
||||
- "Deterministic string->color hashing (32-bit char-code hash mod palette length) for tag badges — avoids a stored color column per D-07"
|
||||
- "Render-time state must avoid setState-in-effect and ref reads during render (react-hooks/set-state-in-effect, react-hooks/refs) — local component state should be derived/reset via event handlers (startEdit/cancel/commit), not synced from props via effects"
|
||||
|
||||
key-files:
|
||||
created:
|
||||
- src/components/ui/editable-cell.tsx
|
||||
- src/components/ui/tag-multi-select.tsx
|
||||
modified: []
|
||||
|
||||
key-decisions:
|
||||
- "Removed the plan's prescribed value-sync useEffect (and a follow-up render-time ref-read attempt) from EditableCell because both violate this project's react-hooks lint rules (set-state-in-effect, refs-during-render). tempValue is now only (re)initialized in startEdit()/cancel(), and the toggle display branch reads `value` directly instead of `tempValue` since tempValue is only meaningful while isEditing=true."
|
||||
|
||||
patterns-established:
|
||||
- "EditableCell / TagMultiSelect are pure presentation primitives with zero ServiceTable-specific logic — Plan 04 imports and wires them directly into ServiceRow/ServiceTable"
|
||||
|
||||
requirements-completed: [OFFER-07, OFFER-08]
|
||||
|
||||
# Metrics
|
||||
duration: ~9min
|
||||
completed: 2026-06-13
|
||||
---
|
||||
|
||||
# Phase 11 Plan 3: EditableCell + TagMultiSelect Shared UI Primitives Summary
|
||||
|
||||
**Two new pure client components — `EditableCell` (click-to-edit text/number/textarea/toggle) and `TagMultiSelect` (hash-colored tag badges with inline add/remove via Plan 02's server actions) — ready for Plan 04 to wire into the database-view `ServiceTable`.**
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** ~9 min
|
||||
- **Started:** 2026-06-13T13:43:00Z
|
||||
- **Completed:** 2026-06-13T13:52:19Z
|
||||
- **Tasks:** 2 of 2
|
||||
- **Files modified:** 2 (both created)
|
||||
|
||||
## Accomplishments
|
||||
|
||||
- `src/components/ui/editable-cell.tsx`: `EditableCell` renders a display-mode div (plain text, "—" for empty, "✓ Attivo"/"✗ Disattivato" for toggle) that becomes an auto-focused `Input`/`Textarea`/checkbox on click. Enter (non-textarea) and blur call `onSave(tempValue)`; Escape reverts without saving. `required` fields show an inline "Campo richiesto" error and block save. `disabled` cells show `cursor-not-allowed opacity-50` and ignore clicks. Toggle type saves immediately on checkbox change (D-14).
|
||||
- `src/components/ui/tag-multi-select.tsx`: `TagMultiSelect` renders tag `Badge`s colored via `getTagColorIndex()` (32-bit char-code hash mod a 7-entry `bg-*-100 text-*-900` pastel palette — D-07, no DB color column). Clicking the cell toggles an inline "+" dropdown; typing a name and pressing Enter (or clicking "+") calls `addTagToService(serviceId, name)`, clears the input, and fires `onTagsChanged`. Clicking a badge's "X" calls `removeTagFromService` (with `stopPropagation` so it doesn't reopen the dropdown). The dropdown closes on outside-click and Escape; `useTransition` disables inputs/buttons during in-flight requests (T-11-13).
|
||||
- Both files pass `npx tsc --noEmit` (zero errors) and `npx eslint` (zero issues) project-wide.
|
||||
|
||||
## Task Commits
|
||||
|
||||
Each task was committed atomically:
|
||||
|
||||
1. **Task 1: Build EditableCell component (text/number/textarea/toggle)** - `3514a37` (feat)
|
||||
2. **Fix: avoid setState/ref access during render in EditableCell** - `55276c1` (fix, Rule 1)
|
||||
3. **Task 2: Build TagMultiSelect component with deterministic color hashing** - `a567a90` (feat)
|
||||
|
||||
**Plan metadata:** (this commit, following SUMMARY)
|
||||
|
||||
## Files Created/Modified
|
||||
|
||||
- `src/components/ui/editable-cell.tsx` - `EditableCell` component + `EditableCellProps`: click-to-edit text/number/textarea/toggle cell, ring-1 focus, Enter/blur save, Escape cancel, required validation, disabled styling
|
||||
- `src/components/ui/tag-multi-select.tsx` - `TagMultiSelect` component + `TagMultiSelectProps` + exported `getTagColorIndex`: hash-colored tag badges, inline add/remove dropdown wired to `addTagToService`/`removeTagFromService`
|
||||
|
||||
## Decisions Made
|
||||
|
||||
- **[Rule 1 - Bug/Lint] Removed value-sync `useEffect` and a render-time ref-read from `EditableCell`.** The plan's prescribed code included a `useEffect(() => setTempValue(String(value)), [value])` to keep `tempValue` aligned with external prop changes. This violates `react-hooks/set-state-in-effect` (calling setState synchronously in an effect body). The first fix attempt (render-time conditional `setTempValue` + `useRef` to track the previous value) instead violated `react-hooks/refs` ("Cannot access refs during render" — this project's eslint config enforces the React Compiler rule that ref reads/writes may not happen during render). Final fix: removed both the effect and the ref-tracking entirely. `tempValue` is now only (re)initialized in `startEdit()` (on entering edit mode) and `cancel()` (on Escape); the toggle display branch (`type === "toggle"`) was changed to read `String(value) === "true"` directly instead of `tempValue === "true"`, since `tempValue` is only meaningful while `isEditing === true`. This is behaviorally equivalent for all 8 spec'd test behaviors — display mode never reads `tempValue` except for the toggle case, which now reads `value` directly.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 1 - Bug/Lint] Fixed `react-hooks/set-state-in-effect` and `react-hooks/refs` violations in EditableCell**
|
||||
|
||||
- **Found during:** Task 1 (post-write `npx eslint` check, run alongside Task 2's verification)
|
||||
- **Issue:** The plan's prescribed `EditableCell` code included a `useEffect` that called `setTempValue(String(value))` whenever the `value` prop changed — this is flagged as an error by `react-hooks/set-state-in-effect` under this project's eslint config (React 19 / eslint-plugin-react-hooks with React Compiler rules). A first fix attempt (render-time conditional state update guarded by a `useRef` storing the previous value) then violated `react-hooks/refs` ("Cannot access refs during render").
|
||||
- **Fix:** Removed both the effect and the ref-tracking. `tempValue` is initialized from `value` only in `startEdit()` and `cancel()` (both already present in the plan's code). The toggle display branch now reads `String(value) === "true"` instead of `tempValue === "true"`.
|
||||
- **Files modified:** `src/components/ui/editable-cell.tsx`
|
||||
- **Verification:** `npx tsc --noEmit` (zero errors) and `npx eslint src/components/ui/editable-cell.tsx src/components/ui/tag-multi-select.tsx` (zero issues) both pass. All 8 behavior tests from the plan's `<behavior>` block remain satisfied: display mode renders `value`/`formatDisplay(value)` (toggle reads `value` directly), click enters edit mode with `tempValue = String(value)`, Enter/blur calls `onSave(tempValue)`, Escape reverts `tempValue` to `String(value)` without calling `onSave`, required validation blocks save with inline error, disabled prevents edit-mode entry, toggle saves immediately on change.
|
||||
- **Committed in:** `55276c1` (separate fix commit, immediately after `3514a37`)
|
||||
|
||||
---
|
||||
|
||||
**Total deviations:** 1 auto-fixed (Rule 1 - lint/bug fix, no behavioral change)
|
||||
**Impact on plan:** None on functionality or design intent — purely a lint-compliance fix required by this project's stricter `eslint-plugin-react-hooks` (React Compiler) ruleset, which the plan's inline code sample did not anticipate.
|
||||
|
||||
## Issues Encountered
|
||||
|
||||
None beyond the lint fix documented above.
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None - both components are fully wired to Plan 02's server actions (`addTagToService`, `removeTagFromService`) and accept generic `onSave`/`onTagsChanged` callbacks from the consumer (Plan 04). No hardcoded/mock data.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None - both components are presentation-only, consuming Plan 02's already-reviewed and admin-gated server actions exactly as specified in this plan's threat model (T-11-11, T-11-12, T-11-13), with no new endpoints, auth paths, or trust-boundary changes.
|
||||
|
||||
## User Setup Required
|
||||
|
||||
None - no new environment variables or external services. (The pending `tags` table migration apply, tracked since Plan 01, remains the only outstanding DB-state item and does not block this plan's code-level/typecheck verification.)
|
||||
|
||||
## Next Phase Readiness
|
||||
|
||||
- **Components ready for Plan 04:** `EditableCell` and `TagMultiSelect` are exported, typecheck cleanly, pass lint, and have zero `ServiceTable`-specific logic — Plan 04 can import both directly into `ServiceRow`/`ServiceTable` for the database-view rewrite.
|
||||
- **`getTagColorIndex` exported** for potential reuse/testing in Plan 04 or beyond.
|
||||
- **Still pending (carried from Plans 01/02):** the `tags` table migration has not yet been applied to the live Postgres DB (firewalled, requires SSH+docker exec by the user). This plan's components are verified via `npx tsc --noEmit`/`npx eslint` only — end-to-end runtime verification (actually adding/removing a tag and seeing it persist) will require that migration to be applied first, same blocker as documented in `11-01-SUMMARY.md` and `11-02-SUMMARY.md`.
|
||||
|
||||
---
|
||||
*Phase: 11-catalog-database-view-ux-legacy-consolidation*
|
||||
*Completed: 2026-06-13*
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- FOUND: src/components/ui/editable-cell.tsx
|
||||
- FOUND: src/components/ui/tag-multi-select.tsx
|
||||
- FOUND: .planning/phases/11-catalog-database-view-ux-legacy-consolidation/11-03-SUMMARY.md
|
||||
- FOUND commit: 3514a37
|
||||
- FOUND commit: 55276c1
|
||||
- FOUND commit: a567a90
|
||||
@@ -28,3 +28,18 @@ Items discovered during execution that are out of scope for the current plan/tas
|
||||
- **Impact:** Low for this plan (doesn't block 11-01 deliverables), but `roadmap update-plan-progress` is a no-op for the rest of v2.1 until this is fixed, and any future agent reading `@.planning/ROADMAP.md` for phase goals/success-criteria (as referenced in `11-CONTEXT.md` canonical_refs) will find nothing.
|
||||
- **Recommended fix:** Populate `.planning/ROADMAP.md` with the v2.1 roadmap content (Phases 11-17), likely should have been written during `/gsd-plan-phase` v2.1 setup (commit `03898f2` "docs(planning): v2.1 milestone setup + Phase 11 context/patterns" added `11-CONTEXT.md`/`11-PATTERNS.md` but apparently not `ROADMAP.md` body content).
|
||||
- **Status:** Not fixed — pre-existing planning-artifact gap, out of scope for this execution plan.
|
||||
|
||||
### 4. `gsd-sdk` CLI not available in 11-03 execution environment
|
||||
|
||||
- **Found during:** 11-03, state-update step (`init.execute-phase`, `state.advance-plan`, `roadmap.update-plan-progress`, etc.)
|
||||
- **Issue:** `gsd-sdk` was not found on `PATH`, and no `node_modules`-installed copy or local CLI was discoverable. All state updates (`STATE.md` position/progress/metrics/decisions/session) for this plan were applied via direct `Edit` to `.planning/STATE.md` instead of via SDK query handlers. `ROADMAP.md`/`REQUIREMENTS.md` updates were skipped: `ROADMAP.md` is still the `PLACEHOLDER` (item #3 above, no Phase 11 section to update), and `REQUIREMENTS.md` already shows OFFER-07/OFFER-08 as "Complete" (marked during 11-02's execution, before this plan's UI work existed — see item #5).
|
||||
- **Impact:** Low for this plan — `STATE.md` was updated manually with equivalent content. No data loss. Future plans (11-04) should check `gsd-sdk` availability early and fall back to manual `STATE.md` edits if still missing.
|
||||
- **Status:** Not fixed — environment/tooling gap, out of scope for this execution plan.
|
||||
|
||||
### 5. OFFER-07/OFFER-08 marked "Complete" in REQUIREMENTS.md before their UI was built
|
||||
|
||||
- **Found during:** 11-03, requirements-check step
|
||||
- **Issue:** `.planning/REQUIREMENTS.md` shows OFFER-07 ("L'utente vede ed edita il catalogo `services` come tabella con inline editing") and OFFER-08 (tag multi-select) as `Complete`, attributed to Phase 11 — but as of 11-02's completion, only the query layer + server actions existed (no UI). 11-03 (this plan) built the `EditableCell`/`TagMultiSelect` primitives, and 11-04 (not yet executed) is the plan that wires them into `ServiceTable` to deliver the actual user-facing inline-edit/tag UX described by OFFER-07/08.
|
||||
- **Impact:** Low — both requirements legitimately complete once 11-04 lands (this plan is a direct, immediate prerequisite, same wave sequence, no risk of the milestone closing prematurely mid-phase). However the "Complete" status in REQUIREMENTS.md was technically premature at the time 11-02 marked it.
|
||||
- **Recommended fix:** No action needed if 11-04 executes next as planned. If 11-04 is ever skipped/deferred, re-open OFFER-07/08 status in REQUIREMENTS.md to "In Progress" until the UI wiring lands.
|
||||
- **Status:** Not fixed — pre-existing from 11-02, informational only, expected to self-resolve when 11-04 completes.
|
||||
|
||||
Reference in New Issue
Block a user