feat(layouts): inline property editing in list and spreadsheet#231
Merged
Conversation
Property cells were read-only links — every change meant opening the detail page. State, priority, assignees, labels, and start/due dates can now be edited in place in the list rows and spreadsheet cells. - EditableCells: state/priority/assignee/label cells that wrap the existing read-only display cells in the shared Dropdown picker; dates reuse DatePickerTrigger. A single openId per layout keeps one picker open at a time. - IssueLayoutTypes: new optional onUpdateIssue(issueId, patch) + IssueInlinePatch. When omitted, cells stay read-only (calendar/gantt/board unaffected). - IssueLayoutList: property pills moved out of the row's navigating <Link> into an interactive sibling region (whole-row hover preserved) so editing a pill no longer navigates. - IssueLayoutSpreadsheet: each property column becomes an editable cell. - IssueListPage: a per-issue serialized persistIssueUpdate (chain + sequence token + route guard) backs both inline edits and board drag-to-column, with optimistic update and refetch-on-failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds inline editing for work-item state, priority, assignee, label, and date fields across list, board, and spreadsheet layouts. It also introduces shared patch types, editable picker cells, overdue-date helpers, and optimistic per-issue update persistence. ChangesInline work-item editing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/work-item/layouts/IssueLayoutList.tsx`:
- Around line 252-263: The editable due-date branch in IssueLayoutList is
replacing DueDateCell and losing overdue/completed/cancelled status styling;
update the onUpdateIssue path so it still preserves the same due-date semantics
while remaining editable. Use the existing DueDateCell behavior as the reference
point, and make the DatePickerTrigger in IssueLayoutList derive and display the
same status cue from issue, issueState, and now instead of rendering a plain
picker-only cell.
In `@apps/web/src/components/work-item/layouts/IssueLayoutSpreadsheet.tsx`:
- Around line 145-156: The editable due date branch in IssueLayoutSpreadsheet
currently replaces DueDateCell with DatePickerTrigger, which removes overdue
highlighting and the completed/cancelled exception. Update the onUpdateIssue
path to preserve the same status cues by either reusing DueDateCell behavior
around DatePickerTrigger or passing the issue state/now-based styling into the
editable control, so spreadsheet mode keeps the overdue indicator when dates are
editable.
In `@apps/web/src/pages/IssueListPage.tsx`:
- Around line 760-762: The board layout still lacks inline edit wiring, so
IssueLayoutBoard remains read-only while the list and spreadsheet layouts
already support onUpdateIssue. Update the IssueListPage render path for
IssueLayoutBoard to pass the same inline update handler used by
handleInlineUpdate, and then thread that prop through IssueLayoutBoard so board
cards can edit issue properties in place alongside onCardMove.
- Around line 116-120: The per-issue PATCH sequencing in IssueListPage is
incorrectly suppressing older failures when a later update succeeds, which can
leave stale optimistic state in local UI. Update the issue update flow that uses
issueUpdateChains and issueUpdateSeq so each failed PATCH still triggers a
refetch or rollback for its own issue even if its seq is no longer the latest,
while keeping the existing per-issue serialization for in-order commits. Ensure
the logic around the update handler in IssueListPage does not gate failure
recovery solely on the current seq token.
- Around line 602-621: `persistIssueUpdate` is using `routeKeyRef.current` as a
navigation guard, but the ref can be stale until the passive effect runs,
allowing a failed `issueService.update` to refetch the previous project’s list
after a route change. Update `routeKeyRef.current` synchronously with the
current `workspaceSlug/projectId` route key, or move that ref sync into
`useLayoutEffect`, so the guard in `persistIssueUpdate` always compares against
the latest route before calling `refetchIssues()`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b3252ab-3658-48e3-b1d9-3112e397e976
📒 Files selected for processing (5)
apps/web/src/components/work-item/EditableCells.tsxapps/web/src/components/work-item/layouts/IssueLayoutList.tsxapps/web/src/components/work-item/layouts/IssueLayoutSpreadsheet.tsxapps/web/src/components/work-item/layouts/IssueLayoutTypes.tsapps/web/src/pages/IssueListPage.tsx
- preserve the overdue cue on editable due-date cells (list, spreadsheet, board) via a shared isOverdue helper and a danger tone on the date trigger. - reconcile after failures: a per-issue chain that fails now re-fetches just that issue once the chain drains, so an older rejected PATCH can't leave a stale optimistic value even when a newer update to another field succeeds. - update the route-guard ref in a layout effect (pre-paint) instead of a passive effect, so a late failure can't reconcile against a stale route. - add inline property editing to board cards too (priority, state, due, labels, assignees) via a CellGuard that stops the card's Link navigation and drag while a picker is used. - move isOverdue to lib/issueRowHelpers (pure helper) to satisfy react-refresh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/work-item/layouts/IssueLayoutBoard.tsx (1)
347-434: 🎯 Functional Correctness | 🟠 MajorMove editable controls out of the card
<Link>
BoardCardstill wraps the editable cells (EditablePriorityCell,EditableStateCell,EditableLabelCell,EditableAssigneeCell) andDatePickerTriggerinside an anchor. Those render<button>triggers and an<input type="date">, which is invalid interactive nesting and can break focus/keyboard behavior.CellGuardonly stops propagation; it doesn’t fix the markup. Split navigation from editing, or make the card container non-anchor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/work-item/layouts/IssueLayoutBoard.tsx` around lines 347 - 434, The editable controls in IssueLayoutBoard are still rendered inside the BoardCard Link, which creates invalid interactive nesting for the editable cells and DatePickerTrigger. Move the editing UI for EditablePriorityCell, EditableStateCell, EditableLabelCell, EditableAssigneeCell, and DatePickerTrigger out of the card anchor, or switch the card container to a non-link wrapper and keep navigation separate. Use the BoardCard/IssueLayoutBoard structure and the CellGuard-wrapped editable sections to locate and refactor the layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/lib/issueRowHelpers.ts`:
- Line 42: The isOverdue state check in issueRowHelpers currently only excludes
completed and cancelled, so issues grouped as canceled can still be marked
overdue. Update the isOverdue logic to treat both spellings of the canceled
state group the same way as completed, using the existing stateGroup check in
issueRowHelpers so callers that normalize either canceled or cancelled are
handled consistently.
---
Outside diff comments:
In `@apps/web/src/components/work-item/layouts/IssueLayoutBoard.tsx`:
- Around line 347-434: The editable controls in IssueLayoutBoard are still
rendered inside the BoardCard Link, which creates invalid interactive nesting
for the editable cells and DatePickerTrigger. Move the editing UI for
EditablePriorityCell, EditableStateCell, EditableLabelCell,
EditableAssigneeCell, and DatePickerTrigger out of the card anchor, or switch
the card container to a non-link wrapper and keep navigation separate. Use the
BoardCard/IssueLayoutBoard structure and the CellGuard-wrapped editable sections
to locate and refactor the layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 889c81c6-324d-4b6d-abb8-ac6283533800
📒 Files selected for processing (7)
apps/web/src/components/work-item/DatePickerTrigger.tsxapps/web/src/components/work-item/IssueRowCells.tsxapps/web/src/components/work-item/layouts/IssueLayoutBoard.tsxapps/web/src/components/work-item/layouts/IssueLayoutList.tsxapps/web/src/components/work-item/layouts/IssueLayoutSpreadsheet.tsxapps/web/src/lib/issueRowHelpers.tsapps/web/src/pages/IssueListPage.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/work-item/layouts/IssueLayoutList.tsx
- apps/web/src/components/work-item/layouts/IssueLayoutSpreadsheet.tsx
CodeRabbit: isOverdue only skipped "cancelled"/"completed", but the API's state group uses the "canceled" spelling, so cancelled issues with a past due date still flagged overdue. Accept both spellings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
Closes #176. Property cells were read-only links — every change meant opening the detail page. State, priority, assignees, labels, and start/due dates are now editable in place in the list rows and spreadsheet cells.
How
EditableCells.tsx(new) —EditableStateCell/EditablePriorityCell/EditableAssigneeCell/EditableLabelCellwrap the existing read-only display cells (StatePill,PriorityIcon,WorkItemAvatarGroup,LabelChips) in the sharedDropdownpicker. Dates reuseDatePickerTrigger. A singleopenIdper layout keeps only one picker open at a time.IssueLayoutTypes— new optionalonUpdateIssue(issueId, patch)+IssueInlinePatch. When omitted, cells render read-only (calendar/gantt/board unaffected).IssueLayoutList— property pills moved out of the row's navigating<Link>into an interactive sibling region (whole-row hover preserved) so editing a pill no longer triggers navigation.IssueLayoutSpreadsheet— each property column becomes an editable cell.IssueListPage— a per-issue serializedpersistIssueUpdate(PATCH chain + sequence token + route guard) backs both inline edits and the board drag-to-column, with optimistic update and refetch-on-failure.Board cards already support changing state via drag-and-drop (#175); full board-card inline pickers are intentionally out of scope here to avoid conflicting with the card drag interaction.
Testing
tsc -b+ ESLint clean.AI assistance
Produced with the help of Claude Code (Claude Opus 4.8); AI-assisted commits carry a
Co-Authored-Bytrailer.Summary by CodeRabbit