WH-058: Post-creation action affordances for cards#73
Conversation
![review-hero[bot]](https://avatars.githubusercontent.com/in/2896273?s=60&v=4){kind=small}
| const journeyMenuRef = useRef<HTMLDivElement>(null) | ||
| const [journeyPos, setJourneyPos] = useState<{ top: number; right: number } | null>(null) | ||
|
|
||
| const statusOptions = useMemo<PropertyOption[]>(() => ( |
There was a problem hiding this comment.
[Bugs & Correctness] critical
Local state (status, priority, teamId, assigneeId) is initialised from card props but never syncs when props change. If the parent re-fetches the card (e.g. after another user edits it, or after navigation), the PropertiesBar will show stale values. The old CardTab had the same pattern, but now that these controls live in a persistent bar (not unmounted on tab switch), the staleness window is wider. Add useEffect hooks to sync local state when card.status, card.priority, etc. change, or key the component on card.id so it remounts on card change.
| ) { | ||
| setJourneyOpen(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
[Bugs & Correctness] suggestion
The handleUpdate function fires-and-forgets the server action inside startTransition with no error handling. If updateCard throws (network error, validation failure), the local state will have already been optimistically updated but the user gets no feedback that the save failed. Consider catching the error and reverting local state, or showing a toast.
|
|
||
| const hasJourney = journalEntries.length > 0 || activeStep !== null | ||
|
|
||
| return ( |
There was a problem hiding this comment.
[Bugs & Correctness] suggestion
journeyPos is calculated once when the dropdown opens and never recalculated on scroll or resize. If the page scrolls or the window resizes while the portal dropdown is open, it will be mispositioned. The old JourneyBar avoided this by using a relative overlay rather than a fixed-position portal. Consider adding a scroll/resize listener that updates journeyPos, or use a relative positioning strategy.
| @@ -0,0 +1,388 @@ | |||
| 'use client' | |||
There was a problem hiding this comment.
[Design & Architecture] suggestion
PropertiesBar merges two unrelated concerns: card property editing (status/priority/team/assignee) and journey progress tracking (journal entries, scheduled steps, suggestions). These have different data dependencies, different interaction patterns, and different reasons to change. The old design had JourneyBar as its own component — the property dropdowns should similarly be their own component, with PropertiesBar composing both. As-is, this 388-line file will grow every time either concern evolves.
| const [status, setStatus] = useState(card.status) | ||
| const [priority, setPriority] = useState(card.priority) | ||
| const [teamId, setTeamId] = useState(card.team.id) | ||
| const [assigneeId, setAssigneeId] = useState(card.assignee?.id ?? '') |
There was a problem hiding this comment.
[Design & Architecture] suggestion
The local state for status/priority/teamId/assigneeId duplicates server state and is updated optimistically via handleUpdate, but there's no error handling — if updateCard fails, the UI shows stale local state that diverged from the database. The old CardTab had the same pattern, but moving it to a persistent bar (visible across all views) increases the blast radius. Consider either adding rollback on failure or using the server response to reconcile.
| </div> | ||
| ) | ||
| } | ||
| export {} |
There was a problem hiding this comment.
[Design & Architecture] suggestion
Replacing the entire module body with export {} leaves a dead file in the tree. If JourneyBar is no longer used, delete the file outright rather than leaving a hollow export that will confuse future readers and show up in import autocompletion.
| <PropertyDropdown | ||
| trigger={ | ||
| <> | ||
| <StatusDot state={statusToDotState(status)} /> |
There was a problem hiding this comment.
[Project Conventions] suggestion
The bar height h-9 (36px) is not on the 4px spacing grid defined in the design system (base unit: 4px, all values multiples of 4). Should be h-8 (32px) or h-10 (40px). Given topbar is 52px and this is a compact properties strip, h-8 (32px) is likely the right fit.
|
🦸 Review Hero Summary Below consensus threshold (3 unique issues not confirmed by majority)
Nitpicks
Local fix prompt (copy to your coding agent)Fix these issues identified on the pull request. One commit per issue fixed.
|
1 participant
Summary
On card creation, want to be able to get straight into something like "create and implement", "create and fix", "create and draft specs", "create and begin interview", "create and workshop ideas", etc.
Should this be an affordance? Should the create card button have a split dropper that has alternative actions? Should there be auto detection of what you might want to do and it just starts that? Workshop this with me. Maybe it's like somewhere between a checkbox "get started" to the left of "create card", but where get started can be changed to start with different things. Don't like the words get started btw, don't use them. Still needs workshopping
Journey
Specs
.workhorse/specs/card-navigation.md.workhorse/specs/card-tab.md.workhorse/specs/workflow-orchestration.md.workhorse/specs/agent-sdk-session.md.workhorse/specs/auto-review.md.workhorse/specs/overview.mdChanged files
src/components/card/CardWorkspace.tsxsrc/components/card/PropertiesBar.tsxsrc/components/card/CardDetailPage.tsxsrc/components/card/CardTab.tsxTest plan
Review Hero