Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions .agents/docs/components-sidebar-session-tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,30 @@ opener is ranked by its freshest opened Session. Without that, nesting would bur
just-updated row under a stale opener and silently break the ordering contract of
Updated mode.

## The leading slot
## The leading slot, and why status left it

The opener and unrelated top-level rows keep the exact flat-list alignment; only a child
widens the shared leading slot from 14px to 26px, which produces the 12px title indent
without shifting the row background. Status outranks the tree because a node can show
only one thing at its centre: an active child drops its trunk and elbow, and an active
opener drops its disclosure. Gating the opener on the whole activity set rather than
`isWorking` alone matters because the disclosure branch replaces the indicator — an
unread opener would otherwise render a chevron and lose its unread dot. The context
menu's expand/collapse item is then the only way to fold a busy opener, so it must stay
wired.
without shifting the row background.

That slot used to hold status as well, with status winning: an active child dropped its
trunk and elbow, and an active opener dropped its disclosure, because a node can show
only one thing at its centre. The cost of that rule was backwards. Working, unread, and
waiting rows are precisely the ones a user is tracking through a tree, and they were the
only rows whose nesting silently disappeared — a group of opened Sessions looked like a
group right up to the moment any of it started running, and then looked like a flat list
with a stray indent.

So status moved to the row's END slot instead, where it replaces the resting metric
cluster (line diff, `Mergeable`, worktree glyph, PR icon, time) for as long as it lasts.
Both halves of that are deliberate. The leading slot now draws the tree unconditionally,
so nesting is a stable fact about the list rather than a function of activity. And the
end slot still shows one thing at a time: a running row reads `[├─ title ......... ◌]`,
with nothing competing for the right edge and more room for the title. Nothing is lost —
the diff, branch, and PR detail were already in the desktop hover info card, which is
where a user goes when they want numbers rather than a glance.

The mobile chat rows still keep status on their leading node; their geometry (48px
indent, a selection checkbox in the same slot, a chevron that has to sit outside the row
button and lift above the edge-back swipe zone) is different enough that the change is a
separate one to make.
24 changes: 15 additions & 9 deletions packages/components/src/components/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,21 @@ Reasoning: [components-sidebar-session-tree.md](../../../../.agents/docs/compone
"simplify" that by rewriting `openedBySessionId` to the root: "Go to Opener Session"
and the conversation's "Opened by" entry must land on the exact Tab that created it.
- The opener and unrelated top-level rows keep the exact flat-list alignment. The shared
leading slot owns the node-centre affordance: an idle opener shows its disclosure at
rest and swaps it for ⋯ on row hover; an idle child shows ├/└ and swaps those for ⋯ in
the SAME 7px-centred position. STATUS OUTRANKS THE TREE on both sides: an active
(working / unread / waiting) child drops the trunk and elbow, and an active opener
drops its disclosure — never both. Gate the opener on the whole activity set, not just
`isWorking`, and keep the context menu's expand/collapse item wired to the same toggle
callback so a busy opener stays foldable. Only a child widens that slot from 14px to
26px, producing the 12px title indent without shifting the row background. Keep
connector geometry in `sidebar-row-shared.tsx`.
leading slot owns the node-centre affordance: an opener shows its disclosure at rest
and swaps it for ⋯ on row hover; a child shows ├/└ and swaps those for ⋯ in the SAME
7px-centred position. It draws the tree UNCONDITIONALLY — a working / unread / waiting
row must never lose its nesting, which is the whole point of moving status out of it.
Only a child widens that slot from 14px to 26px, producing the 12px title indent
without shifting the row background. Keep connector geometry in
`sidebar-row-shared.tsx`, and keep the context menu's expand/collapse item wired to the
same toggle callback.
- Desktop row status (working / waiting / unread) belongs to the END slot
(`SessionRowStatusIndicator` inside `SidebarRowEndSlot`) and nowhere else. While a
status shows it REPLACES that slot's resting content — line diff, `Mergeable`, worktree
glyph, PR icon, mobile time — so the right edge is one 14px mark; the metrics stay one
hover away in the desktop info card. Pass the three flags to the end slot, never to the
leading slot. The mobile chat rows keep their own leading-node rule
([mobile/AGENTS.md](mobile/AGENTS.md)); do not assume the two match.
- The resolver needs `allActiveSessions`, so any new list must take it from the sidebar
rather than re-deriving it from rows.
- The tree never hides a Session: a missing, cross-section, cross-group, cycling, or
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/components/loro-app-sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -698,9 +698,6 @@ const LocalProjectSessionItem = memo(function LocalProjectSessionItem({
>
<div className="flex items-center gap-1.5">
<SessionRowLeadingSlot
isWaitingPermission={isWaitingPermission}
isWorking={isWorking}
hasUnreadMessages={hasUnreadMessages}
showMenuButton={hasContextMenuActions}
menuLabel={moreActionsLabel}
openedByTree={openedByTree}
Expand Down Expand Up @@ -733,6 +730,9 @@ const LocalProjectSessionItem = memo(function LocalProjectSessionItem({
has one, with a faint worktree glyph just to its left; the Archive
button replaces it on desktop hover. */}
<SidebarRowEndSlot
isWaitingPermission={isWaitingPermission}
isWorking={isWorking}
hasUnreadMessages={hasUnreadMessages}
restIcon={
showPr || showWorktreeIcon ? (
<span className="flex items-center gap-1.5">
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/components/session-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -952,9 +952,6 @@ const SessionGroupSection = memo(function SessionGroupSection({
) : null}
<div className="flex min-w-0 items-center gap-1.5">
<SessionRowLeadingSlot
isWaitingPermission={session.isWaitingPermission}
isWorking={session.isWorking}
hasUnreadMessages={session.hasUnreadMessages}
showMenuButton={hasMenuActions}
menuLabel={moreActionsLabel}
openedByTree={openedByTreeSlot}
Expand Down Expand Up @@ -986,6 +983,9 @@ const SessionGroupSection = memo(function SessionGroupSection({
</div>
{/* Keep PR at the right edge, with All Changes totals immediately before it. */}
<SidebarRowEndSlot
isWaitingPermission={session.isWaitingPermission}
isWorking={session.isWorking}
hasUnreadMessages={session.hasUnreadMessages}
restIcon={
isChatSession ? (
<span className={cn('flex items-center gap-1.5', useAnchor && 'z-20')}>
Expand Down
142 changes: 86 additions & 56 deletions packages/components/src/components/sidebar-row-shared.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,24 @@ import { getGitHubOwnerAvatarUrl } from '@/lib/github-avatar';
* flat Updated list in `sidebar-updated-task-list.tsx`) render the same anatomy,
* so the pieces live here once instead of being copied three times.
*
* Row anatomy: `[① status/tree affordance | more][② author avatar? + title][③ diff/mergeable?][worktree?][④ PR icon? | archive]`.
* Row anatomy: `[① tree affordance | more][② author avatar? + title][③ status | diff/mergeable? + worktree? + PR icon? | archive]`.
* The author avatar (`SessionRowAuthorAvatar`) only appears in team ("All Tasks") scope on a
* multi-member workspace; otherwise the title owns the leading edge of slot ②. The leading slot
* stays reserved even when empty (the ⋯ menu button reveals there on hover). A local worktree
* session shows a faint worktree glyph (`SessionRowWorktreeIndicator`) inside the metric cluster,
* between the line diff and the PR icon (taking the PR's right-edge spot when there is no PR);
* GitHub sessions are always worktrees so they never show it. The full
* repo / folder / worktree session-type detail still lives in the desktop hover info card
* (`session-info-hover-card.tsx`). At rest, PR status
* owns the right edge when present and line diff sits immediately before it. Without
* PR status, line diff owns the right edge. Archive replaces the trailing content on
* hover, so the row's rightmost metric never shifts.
* (`session-info-hover-card.tsx`).
*
* The END slot (③) is the row's single status channel: working / waiting / unread
* REPLACES the whole resting metric cluster there, so an active row reads
* `[title][status]` and nothing else competes with it. Only a resting row shows
* metrics, where PR status owns the right edge when present with the line diff
* immediately before it. Archive replaces the trailing content on hover, so the
* row's rightmost mark never shifts. The leading slot (①) is therefore free to
* ALWAYS draw the opened-by tree: a running or unread child keeps its ├/└ and an
* active opener keeps its disclosure.
*/
export type SidebarRowKind = 'github' | 'local' | 'chat';

Expand Down Expand Up @@ -114,13 +120,18 @@ function MaskedPrCiIcon({
}

/**
* ① The 14px status slot at the row's leading edge. Single-slot priority:
* `waitingPermission > isWorking > hasUnread > empty`. The PR status now lives in
* the end slot (④), not here, so a resting GitHub row's leading slot is empty
* unless it is working / unread — but the slot is always reserved so the ⋯ menu
* button can reveal there on hover without shifting the row.
* ③ The 14px status mark, rendered at the row's TRAILING edge inside
* {@link SidebarRowEndSlot}. Single-mark priority:
* `waitingPermission > isWorking > hasUnread`. Returns `null` when the session is
* idle and read, which is what lets the end slot fall back to its resting metric
* cluster (diff / worktree / PR) — a row shows one or the other, never both.
*
* It lives at the end rather than at the leading edge so the opened-by tree can
* own the leading slot unconditionally: before this, an active child had to drop
* its ├/└ connectors and the nesting silently disappeared exactly on the rows a
* user watches most.
*/
function SessionRowIndicator({
function SessionRowStatusIndicator({
isWaitingPermission,
isWorking,
hasUnreadMessages,
Expand All @@ -144,6 +155,8 @@ function SessionRowIndicator({
icon = <span className="h-2 w-2 rounded-full bg-primary" />;
}

if (!icon) return null;

return (
<div
data-session-row-indicator=""
Expand All @@ -154,6 +167,19 @@ function SessionRowIndicator({
);
}

/** True when {@link SessionRowStatusIndicator} would draw a mark for this row. */
function hasSessionRowStatus({
isWaitingPermission,
isWorking,
hasUnreadMessages,
}: {
isWaitingPermission?: boolean;
isWorking?: boolean;
hasUnreadMessages?: boolean;
}): boolean {
return Boolean(isWaitingPermission || isWorking || hasUnreadMessages);
}

/**
* ③ The PR status icon shown in the final slot (colored, non-interactive —
* opening the PR is handled by the row context menu + info card). Sized to match
Expand Down Expand Up @@ -387,31 +413,27 @@ export function SessionRowOpenedByMenuItems({
}

/**
* ① Status, opened-by tree affordance, and the hover ⋯ button share one spot.
* ① The opened-by tree affordance and the hover ⋯ button share one spot.
* ⋯ synthesizes `contextmenu` so there is no second menu. Hover fades the rest
* state (disclosure, ├/└, or status) without moving the title.
* state (the disclosure or ├/└) without moving the title.
*
* A working opener swaps its disclosure for the status spinner — loading
* outranks folding, so the collapse toggle moves to the row's context menu
* while the session is active.
* The tree is drawn unconditionally: status lives in the END slot
* ({@link SessionRowStatusIndicator}), so a working / unread / waiting row no
* longer has to trade its nesting away for a status mark. A row that is neither
* an opener nor a child still reserves the empty slot, so every title in the list
* shares one x and the ⋯ button has somewhere to appear.
*/
export function SessionRowLeadingSlot({
isWaitingPermission,
isWorking,
hasUnreadMessages,
showMenuButton,
menuLabel,
openedByTree,
/** Fade the status while hovering (e.g. 'group-hover/row:opacity-0' for named groups). */
/** Fade the rest state while hovering (e.g. 'group-hover/row:opacity-0' for named groups). */
fadeClassName = 'group-hover:opacity-0 group-data-[menu-open]:opacity-0',
/** Disable an opener disclosure while its ⋯ replacement is active. */
restPointerClassName = 'group-hover:pointer-events-none group-data-[menu-open]:pointer-events-none',
/** Reveal the ⋯ button while hovering. */
revealClassName = 'group-hover:opacity-100 group-hover:pointer-events-auto group-data-[menu-open]:opacity-100 group-data-[menu-open]:pointer-events-auto',
}: {
isWaitingPermission?: boolean;
isWorking?: boolean;
hasUnreadMessages?: boolean;
showMenuButton?: boolean;
menuLabel: string;
openedByTree?: SessionRowOpenedByTreeSlot;
Expand All @@ -420,30 +442,18 @@ export function SessionRowLeadingSlot({
revealClassName?: string;
}) {
const childTree = openedByTree?.kind === 'child' ? openedByTree : null;
const isTreeChild = childTree !== null;
const hasActivity = Boolean(isWaitingPermission || isWorking || hasUnreadMessages);
const showChildConnectors = childTree !== null && !hasActivity;
/* Status outranks the tree on BOTH sides of the relationship: an active
child drops its ├/└ and an active opener drops its disclosure, because one
node can only say one thing and "this session needs you" beats "this
session has children". Folding stays reachable the same way it always is
on a busy row — hover swaps in ⋯, whose menu carries the same toggle.

Gated on the whole activity set, not just `isWorking`: the disclosure
branch REPLACES the indicator, so an unread or waiting-permission opener
would otherwise render a chevron and silently drop its own status mark. */
const openerTree = openedByTree?.kind === 'opener' && !hasActivity ? openedByTree : null;
const openerTree = openedByTree?.kind === 'opener' ? openedByTree : null;
const restClassName = showMenuButton
? cn('transition-opacity duration-100', fadeClassName)
: undefined;
const controlLeftClassName = isTreeChild ? TREE_CONTROL_LEFT_CLASS : 'left-1/2';
const controlLeftClassName = childTree ? TREE_CONTROL_LEFT_CLASS : 'left-1/2';

return (
<div
data-session-row-leading-slot=""
className={cn(
'relative flex h-3.5 shrink-0 items-center',
isTreeChild ? TREE_CHILD_SLOT_CLASS : 'w-3.5 justify-center'
childTree ? TREE_CHILD_SLOT_CLASS : 'w-3.5 justify-center'
)}
>
{openerTree ? (
Expand Down Expand Up @@ -473,7 +483,7 @@ export function SessionRowLeadingSlot({
aria-hidden="true"
/>
</button>
) : showChildConnectors ? (
) : childTree ? (
<div className={cn('absolute inset-0', restClassName)}>
<span
aria-hidden="true"
Expand All @@ -496,15 +506,7 @@ export function SessionRowLeadingSlot({
)}
/>
</div>
) : (
<div className={restClassName}>
<SessionRowIndicator
isWaitingPermission={isWaitingPermission}
isWorking={isWorking}
hasUnreadMessages={hasUnreadMessages}
/>
</div>
)}
) : null}
{showMenuButton ? (
<button
type="button"
Expand All @@ -524,9 +526,9 @@ export function SessionRowLeadingSlot({
);
}}
className={cn(
// Overlay a 20px hit target centered on the 14px status slot so the ⋯
// Overlay a 20px hit target centered on the 14px leading slot so the ⋯
// gets a visible rounded hover chip (it reads as clickable) without
// the tiny status-slot footprint clipping the background.
// the tiny slot footprint clipping the background.
'absolute top-1/2 z-20 flex h-5 w-5 -translate-x-1/2 -translate-y-1/2 items-center justify-center rounded-md opacity-0 pointer-events-none',
controlLeftClassName,
'text-sidebar-foreground-muted transition-[opacity,color,background-color] duration-100',
Expand Down Expand Up @@ -609,29 +611,57 @@ export function SidebarRowArchiveButton({
}

/**
* ③ The final slot at the row's right edge. Its resting content can be a compact
* line diff, time, or an icon. Archive is absolutely overlaid on hover, so a wide
* diff never causes layout movement. When the rest content is absent but Archive is
* available, it still reserves the action's 20px hit target.
* ③ The final slot at the row's right edge, and the row's ONE status channel.
*
* A working / waiting / unread session shows only its status mark here: the
* resting content (line diff, `Mergeable`, worktree glyph, PR icon, time) is
* dropped for as long as the status lasts. That is deliberate — the status is the
* fact the user is watching, the metrics are still one hover away in the desktop
* info card, and collapsing them keeps the right edge to a single 14px mark
* instead of a cluster that competes with it and eats the title.
*
* Archive is absolutely overlaid on hover, so a wide diff never causes layout
* movement. When the rest content is absent but Archive is available, it still
* reserves the action's 20px hit target.
*/
export function SidebarRowEndSlot({
isWaitingPermission,
isWorking,
hasUnreadMessages,
restIcon,
archive,
/** Fade the rest icon while hovering (match the row's group, e.g. 'group-hover/row:opacity-0'). */
fadeClassName = 'group-hover:opacity-0',
}: {
isWaitingPermission?: boolean;
isWorking?: boolean;
hasUnreadMessages?: boolean;
restIcon?: ReactNode;
archive?: ReactNode;
fadeClassName?: string;
}) {
const hasRest = Boolean(restIcon);
const restContent = hasSessionRowStatus({
isWaitingPermission,
isWorking,
hasUnreadMessages,
}) ? (
Comment on lines +643 to +647

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the private-sharing indicator during activity

When a private task is also unread, working, or waiting for permission, this branch discards the entire restIcon; both task-list.tsx and sidebar-updated-task-list.tsx place SessionSharingIndicator inside that value. Consequently, these common states—especially a never-opened row, which is unread—remove the list's lock and its “who can open this conversation” tooltip, obscuring the session's access scope along with the intentionally suppressed metrics. Keep the privacy indicator outside the replaceable metric cluster so it remains visible beside the activity status.

Useful? React with 👍 / 👎.

<SessionRowStatusIndicator
isWaitingPermission={isWaitingPermission}
isWorking={isWorking}
hasUnreadMessages={hasUnreadMessages}
/>
) : (
restIcon
);
const hasRest = Boolean(restContent);
// Reserve the action hit target whenever something can occupy it; otherwise the
// slot sizes to its resting content (for example, a +/- line diff).
const reserve = hasRest || Boolean(archive);
return (
// pointer-events-none so the resting PR icon area still passes clicks through to
// the row's navigation; the Archive button re-enables pointer events on hover.
<div
data-session-row-end-slot=""
className={cn(
'relative flex h-5 shrink-0 items-center justify-center pointer-events-none',
reserve ? 'min-w-5' : 'w-0'
Expand All @@ -641,7 +671,7 @@ export function SidebarRowEndSlot({
<span
className={cn('flex', archive && cn('transition-opacity duration-100', fadeClassName))}
>
{restIcon}
{restContent}
</span>
) : null}
{archive}
Expand Down
Loading
Loading