feat(FN-2631): revise task detail move split-button behavior
- Rework TaskDetailModal move controls to use split-button primary/secondary transitions with in-review-specific options - Add dedicated split-button styling for divider, chevron menu, focus states, and mobile sizing using design tokens - Keep in-review merge action visible while move options remain accessible through secondary menu actions - Expand TaskDetailModal tests to cover split-button rendering, primary click behavior, and secondary transition menu contents
This commit is contained in:
@@ -591,16 +591,55 @@
|
||||
|
||||
.detail-move-actions-in-review {
|
||||
display: flex;
|
||||
gap: 10px;
|
||||
gap: var(--space-md);
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
.detail-move-split-btn {
|
||||
position: relative;
|
||||
display: flex;
|
||||
align-items: stretch;
|
||||
border-radius: var(--radius-md);
|
||||
}
|
||||
|
||||
.detail-move-split-btn__main {
|
||||
border-top-right-radius: 0;
|
||||
border-bottom-right-radius: 0;
|
||||
}
|
||||
|
||||
.detail-move-split-btn__main:focus-visible,
|
||||
.detail-move-split-btn__chevron:focus-visible {
|
||||
box-shadow: var(--focus-ring-strong);
|
||||
outline: none;
|
||||
}
|
||||
|
||||
.detail-move-split-btn__chevron {
|
||||
border-top-left-radius: 0;
|
||||
border-bottom-left-radius: 0;
|
||||
min-width: calc(var(--space-xs) + var(--space-sm) + var(--space-md));
|
||||
padding: 0;
|
||||
}
|
||||
|
||||
.detail-move-split-btn__divider {
|
||||
width: 1px;
|
||||
height: calc(var(--space-sm) + var(--space-xs));
|
||||
align-self: center;
|
||||
background: var(--border);
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
@media (hover: hover) {
|
||||
.detail-move-split-btn__chevron:hover {
|
||||
background: var(--cta-bg-hover, var(--cta-bg));
|
||||
}
|
||||
}
|
||||
|
||||
.detail-actions-menu,
|
||||
.detail-move-menu {
|
||||
position: absolute;
|
||||
bottom: calc(100% + 4px);
|
||||
bottom: calc(100% + var(--space-xs));
|
||||
z-index: 50;
|
||||
min-width: 140px;
|
||||
min-width: calc(var(--space-2xl) + var(--space-2xl) + var(--space-md));
|
||||
background: var(--surface);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md);
|
||||
@@ -616,11 +655,15 @@
|
||||
right: 0;
|
||||
}
|
||||
|
||||
.detail-move-split-btn__menu {
|
||||
right: 0;
|
||||
}
|
||||
|
||||
.detail-actions-menu-item,
|
||||
.detail-move-menu-item {
|
||||
display: block;
|
||||
width: 100%;
|
||||
padding: 8px 12px;
|
||||
padding: var(--space-sm) var(--space-md);
|
||||
font-size: 13px;
|
||||
font-weight: 400;
|
||||
color: var(--text);
|
||||
@@ -628,7 +671,7 @@
|
||||
border: none;
|
||||
cursor: pointer;
|
||||
text-align: left;
|
||||
transition: background 0.1s;
|
||||
transition: background var(--transition-fast);
|
||||
}
|
||||
|
||||
.detail-actions-menu-item:hover,
|
||||
@@ -642,6 +685,17 @@
|
||||
background: var(--surface-hover);
|
||||
}
|
||||
|
||||
@media (max-width: 768px) {
|
||||
.detail-move-split-btn__main,
|
||||
.detail-move-split-btn__chevron {
|
||||
min-height: calc(var(--space-lg) + var(--space-xl));
|
||||
}
|
||||
|
||||
.detail-move-split-btn__chevron {
|
||||
min-width: calc(var(--space-lg) + var(--space-xl));
|
||||
}
|
||||
}
|
||||
|
||||
.detail-actions-menu-item-danger {
|
||||
color: var(--color-error, #dc3545);
|
||||
}
|
||||
|
||||
@@ -1229,6 +1229,11 @@ export function TaskDetailModal({
|
||||
}, [workingTask.modelProvider, workingTask.validatorModelProvider, workingTask.planningModelProvider]);
|
||||
|
||||
const transitions = VALID_TRANSITIONS[task.column] || [];
|
||||
const inReviewMoveTransitions: Column[] = ["todo", "in-progress"];
|
||||
const moveTransitions = task.column === "in-review" ? inReviewMoveTransitions : transitions;
|
||||
const primaryMoveTransition = moveTransitions[0];
|
||||
const secondaryMoveTransitions = moveTransitions.slice(1);
|
||||
const hasSecondaryMoveOptions = secondaryMoveTransitions.length > 0;
|
||||
const prAutomationStatusLabels: Record<string, string> = {
|
||||
"creating-pr": "Creating PR…",
|
||||
"awaiting-pr-checks": "Awaiting PR checks",
|
||||
@@ -2075,90 +2080,98 @@ export function TaskDetailModal({
|
||||
{/* Move dropdown — column transitions and merge actions */}
|
||||
<div className="detail-move-dropdown" ref={moveMenuRef}>
|
||||
{task.column === "in-review" ? (
|
||||
<>
|
||||
{/* In-review: show merge controls inline with Move dropdown for secondary moves */}
|
||||
<div className="detail-move-actions-in-review">
|
||||
<div className="detail-move-actions-in-review">
|
||||
<div className="detail-move-split-btn">
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
onClick={() => {
|
||||
setShowMoveMenu((prev) => !prev);
|
||||
setShowActionsMenu(false);
|
||||
}}
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={showMoveMenu}
|
||||
className="btn btn-primary btn-sm detail-move-split-btn__main"
|
||||
onClick={() => primaryMoveTransition && handleMoveMenuItemClick(primaryMoveTransition)}
|
||||
disabled={!primaryMoveTransition}
|
||||
>
|
||||
Move
|
||||
<ChevronDown size={12} />
|
||||
Move to {primaryMoveTransition ? COLUMN_LABELS[primaryMoveTransition] : ""}
|
||||
</button>
|
||||
{prAutomationLabel ? (
|
||||
<button className="btn btn-primary btn-sm" disabled>
|
||||
{prAutomationLabel}
|
||||
</button>
|
||||
) : (
|
||||
<button className="btn btn-primary btn-sm" onClick={handleMergeMenuItemClick}>
|
||||
Merge & Close
|
||||
</button>
|
||||
{hasSecondaryMoveOptions && (
|
||||
<>
|
||||
<span className="detail-move-split-btn__divider" aria-hidden="true" />
|
||||
<button
|
||||
className="btn btn-primary btn-sm detail-move-split-btn__chevron"
|
||||
onClick={() => {
|
||||
setShowMoveMenu((prev) => !prev);
|
||||
setShowActionsMenu(false);
|
||||
}}
|
||||
aria-label="More move options"
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={showMoveMenu}
|
||||
>
|
||||
<ChevronDown size={12} />
|
||||
</button>
|
||||
</>
|
||||
)}
|
||||
{showMoveMenu && hasSecondaryMoveOptions && (
|
||||
<div className="detail-move-menu detail-move-split-btn__menu" role="menu">
|
||||
{secondaryMoveTransitions.map((col) => (
|
||||
<button
|
||||
key={col}
|
||||
className="detail-move-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => handleMoveMenuItemClick(col)}
|
||||
>
|
||||
{col === "in-progress" ? "Back to In Progress" : `Move to ${COLUMN_LABELS[col]}`}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
{showMoveMenu && (
|
||||
<div className="detail-move-menu" role="menu">
|
||||
<button
|
||||
className="detail-move-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => handleMoveMenuItemClick("todo")}
|
||||
>
|
||||
Move to Todo
|
||||
</button>
|
||||
<button
|
||||
className="detail-move-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => handleMoveMenuItemClick("in-progress")}
|
||||
>
|
||||
Back to In Progress
|
||||
</button>
|
||||
</div>
|
||||
{prAutomationLabel ? (
|
||||
<button className="btn btn-primary btn-sm" disabled>
|
||||
{prAutomationLabel}
|
||||
</button>
|
||||
) : (
|
||||
<button className="btn btn-primary btn-sm" onClick={handleMergeMenuItemClick}>
|
||||
Merge & Close
|
||||
</button>
|
||||
)}
|
||||
</>
|
||||
</div>
|
||||
) : (
|
||||
/* Other columns: primary action is the first transition, dropdown for more */
|
||||
<>
|
||||
<div className="detail-move-split-btn">
|
||||
<button
|
||||
className="btn btn-primary btn-sm"
|
||||
onClick={() => handleMoveMenuItemClick(transitions[0])}
|
||||
disabled={transitions.length === 0}
|
||||
className="btn btn-primary btn-sm detail-move-split-btn__main"
|
||||
onClick={() => primaryMoveTransition && handleMoveMenuItemClick(primaryMoveTransition)}
|
||||
disabled={!primaryMoveTransition}
|
||||
>
|
||||
Move to {transitions.length > 0 ? COLUMN_LABELS[transitions[0]] : ""}
|
||||
Move to {primaryMoveTransition ? COLUMN_LABELS[primaryMoveTransition] : ""}
|
||||
</button>
|
||||
{transitions.length > 1 && (
|
||||
{hasSecondaryMoveOptions && (
|
||||
<>
|
||||
<span className="detail-move-split-btn__divider" aria-hidden="true" />
|
||||
<button
|
||||
className="btn btn-sm"
|
||||
className="btn btn-primary btn-sm detail-move-split-btn__chevron"
|
||||
onClick={() => {
|
||||
setShowMoveMenu((prev) => !prev);
|
||||
setShowActionsMenu(false);
|
||||
}}
|
||||
aria-label="More move options"
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={showMoveMenu}
|
||||
>
|
||||
<ChevronDown size={12} />
|
||||
</button>
|
||||
{showMoveMenu && (
|
||||
<div className="detail-move-menu" role="menu">
|
||||
{transitions.map((col) => (
|
||||
<button
|
||||
key={col}
|
||||
className="detail-move-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => handleMoveMenuItemClick(col)}
|
||||
>
|
||||
Move to {COLUMN_LABELS[col]}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</>
|
||||
{showMoveMenu && hasSecondaryMoveOptions && (
|
||||
<div className="detail-move-menu detail-move-split-btn__menu" role="menu">
|
||||
{secondaryMoveTransitions.map((col) => (
|
||||
<button
|
||||
key={col}
|
||||
className="detail-move-menu-item"
|
||||
role="menuitem"
|
||||
onClick={() => handleMoveMenuItemClick(col)}
|
||||
>
|
||||
Move to {COLUMN_LABELS[col]}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</>
|
||||
|
||||
@@ -505,7 +505,7 @@ describe("TaskDetailModal", () => {
|
||||
expect(addToast).toHaveBeenCalledWith("Server error", "error");
|
||||
});
|
||||
|
||||
it("shows 'Move to Todo' in Move dropdown for in-review tasks (not 'Retry')", () => {
|
||||
it("shows in-review split button with primary action and secondary move option", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-review" })}
|
||||
@@ -518,21 +518,20 @@ describe("TaskDetailModal", () => {
|
||||
/>,
|
||||
);
|
||||
|
||||
// Should show "Move" button that opens dropdown
|
||||
const moveBtn = screen.getByRole("button", { name: /move/i });
|
||||
expect(moveBtn).toBeTruthy();
|
||||
expect(screen.getByRole("button", { name: "Move to Todo" })).toBeTruthy();
|
||||
const chevronBtn = screen.getByRole("button", { name: "More move options" });
|
||||
expect(chevronBtn).toBeTruthy();
|
||||
|
||||
// Open Move dropdown to see "Move to Todo"
|
||||
fireEvent.click(moveBtn);
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Todo" })).toBeTruthy();
|
||||
fireEvent.click(chevronBtn);
|
||||
expect(screen.getByRole("menuitem", { name: "Back to In Progress" })).toBeTruthy();
|
||||
expect(screen.queryByRole("menuitem", { name: "Move to Todo" })).toBeNull();
|
||||
|
||||
// No Retry in Actions dropdown
|
||||
const actionsBtn = screen.getByRole("button", { name: /actions/i });
|
||||
fireEvent.click(actionsBtn);
|
||||
expect(screen.queryByRole("menuitem", { name: "Retry" })).toBeNull();
|
||||
});
|
||||
|
||||
it("in-review failed task shows both 'Retry' (in Actions) and 'Move to Todo' (in Move dropdown)", async () => {
|
||||
it("in-review failed task shows both Retry action and secondary move option", async () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-review", status: "failed" })}
|
||||
@@ -546,21 +545,109 @@ describe("TaskDetailModal", () => {
|
||||
/>,
|
||||
);
|
||||
|
||||
// Open Actions dropdown and check Retry
|
||||
const actionsBtn = screen.getByRole("button", { name: /actions/i });
|
||||
await act(async () => {
|
||||
fireEvent.click(actionsBtn);
|
||||
});
|
||||
expect(screen.getByRole("menuitem", { name: "Retry" })).toBeTruthy();
|
||||
// Total count: exactly one Retry
|
||||
expect(screen.getAllByRole("menuitem", { name: "Retry" })).toHaveLength(1);
|
||||
|
||||
// Open Move dropdown and check Move to Todo
|
||||
const moveBtn = screen.getByRole("button", { name: /move/i });
|
||||
const chevronBtn = screen.getByRole("button", { name: "More move options" });
|
||||
await act(async () => {
|
||||
fireEvent.click(moveBtn);
|
||||
fireEvent.click(chevronBtn);
|
||||
});
|
||||
expect(screen.getByRole("menuitem", { name: "Back to In Progress" })).toBeTruthy();
|
||||
expect(screen.queryByRole("menuitem", { name: "Move to Todo" })).toBeNull();
|
||||
});
|
||||
|
||||
it("split-button renders with chevron when multiple transitions exist", async () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-progress" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("button", { name: "Move to In Review" })).toBeTruthy();
|
||||
const chevronBtn = screen.getByRole("button", { name: "More move options" });
|
||||
expect(chevronBtn).toBeTruthy();
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(chevronBtn);
|
||||
});
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Todo" })).toBeTruthy();
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Planning" })).toBeTruthy();
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Done" })).toBeTruthy();
|
||||
expect(screen.queryByRole("menuitem", { name: "Move to In Review" })).toBeNull();
|
||||
});
|
||||
|
||||
it("split-button renders without chevron when only one transition", () => {
|
||||
const { container } = render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "triage" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByRole("button", { name: "Move to Todo" })).toBeTruthy();
|
||||
expect(screen.queryByRole("button", { name: "More move options" })).toBeNull();
|
||||
expect(container.querySelector(".detail-move-split-btn__divider")).toBeNull();
|
||||
});
|
||||
|
||||
it("clicking main button executes primary transition immediately", async () => {
|
||||
const onMoveTask = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-progress" })}
|
||||
onClose={noop}
|
||||
onMoveTask={onMoveTask}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: "Move to In Review" }));
|
||||
});
|
||||
|
||||
expect(onMoveTask).toHaveBeenCalledWith("FN-099", "in-review");
|
||||
expect(screen.queryByRole("menu")).toBeNull();
|
||||
});
|
||||
|
||||
it("chevron dropdown includes only secondary transitions", async () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({ column: "in-progress" })}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: "More move options" }));
|
||||
});
|
||||
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Todo" })).toBeTruthy();
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Planning" })).toBeTruthy();
|
||||
expect(screen.getByRole("menuitem", { name: "Move to Done" })).toBeTruthy();
|
||||
expect(screen.queryByRole("menuitem", { name: "Move to In Review" })).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2479,9 +2566,8 @@ describe("TaskDetailModal", () => {
|
||||
|
||||
expect(screen.getByText("Merge & Close")).toBeTruthy();
|
||||
|
||||
// Back to In Progress is now in the Move dropdown
|
||||
const moveBtn = screen.getByRole("button", { name: /move/i });
|
||||
fireEvent.click(moveBtn);
|
||||
// Back to In Progress is in secondary move options
|
||||
fireEvent.click(screen.getByRole("button", { name: "More move options" }));
|
||||
expect(screen.getByRole("menuitem", { name: "Back to In Progress" })).toBeTruthy();
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user