Skip to content
Open
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
111 changes: 111 additions & 0 deletions apps/desktop/e2e/pinned-reorder.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* E2E: reorder pinned sessions by dragging within the sidebar.
*
* Regression for the two-drag-systems conflict (#47728 follow-up): the row body
* is a session-drag source (drag onto chat → link), and reorder used to live on
* a separate, near-invisible dnd-kit grab handle that the session-drag stole
* from. Now one pointer drag routes by DROP LOCATION — released within the
* pinned list it reorders, released on a chat surface it links. This test drives
* a REAL pointer drag (mouse.move/down/up in steps, like tile-unread-bug.spec)
* and asserts the pinned order actually changes.
*
* Prerequisite: `npm run build` must have been run so dist/ exists.
*/

import { expect, test, type Page } from '@playwright/test'

import { type MockBackendFixture, setupMockBackend, waitForAppReady } from './fixtures'
import { restartMockServer } from './mock-server'

/** A pinned row's durable order is read from the DOM: the tagged reorder rows
* in document order carry the session id in data-reorder-row. */
async function pinnedOrder(page: Page): Promise<string[]> {
return page.locator('[data-reorder-row]').evaluateAll(nodes =>
nodes.map(n => (n as HTMLElement).dataset.reorderRow ?? '')
)
}

/** Create a fresh session with a distinct first message so its row is findable. */
async function createSession(page: Page, marker: string): Promise<void> {
await page.locator('button:has-text("New session")').first().click()
await page.waitForTimeout(500)

const composer = page.locator('[contenteditable="true"]').first()
await composer.waitFor({ state: 'visible', timeout: 10_000 })
await composer.click()
await composer.type(marker, { delay: 15 })
await page.keyboard.press('Enter')

await page.waitForFunction(
text => (document.body.textContent ?? '').includes(text),
marker,
{ timeout: 20_000 }
)
}

/** Shift-click a session row to pin it (the row's onClick pin shortcut). */
async function shiftClickPin(page: Page, marker: string): Promise<void> {
const row = page.locator('[data-slot="sidebar"] button').filter({ hasText: marker }).first()
await row.click({ modifiers: ['Shift'] })
await page.waitForTimeout(300)
}

test.describe('sidebar — pinned reorder via pointer drag', () => {
test.describe.configure({ mode: 'serial' })

let fixture: MockBackendFixture

test.beforeAll(async () => {
restartMockServer()
fixture = await setupMockBackend()
await waitForAppReady(fixture, 120_000)
})

test.afterAll(async () => {
await fixture?.cleanup()
})

test('dragging a pinned row within the sidebar reorders it', async () => {
const page = fixture.page

// Two pinned sessions, distinct markers so their rows are addressable.
await createSession(page, 'E2E_PIN_ALPHA')
await shiftClickPin(page, 'E2E_PIN_ALPHA')
await createSession(page, 'E2E_PIN_BETA')
await shiftClickPin(page, 'E2E_PIN_BETA')

// Both rows now carry data-reorder-row (the pinned list is a reorder zone).
await expect
.poll(() => pinnedOrder(page).then(o => o.length), { timeout: 10_000 })
.toBe(2)

const before = await pinnedOrder(page)
expect(before).toHaveLength(2)

const topId = before[0]!
const topRow = page.locator(`[data-reorder-row="${topId}"]`)
const box = await topRow.boundingBox()
expect(box, 'top pinned row must be visible').not.toBeNull()

// Drag the top row down past the second row's midpoint, in steps so the
// pointer session engages (threshold) and resolveMove tracks the slot.
const startX = box!.x + box!.width / 2
const startY = box!.y + box!.height / 2
const targetY = box!.y + box!.height * 1.6

await page.mouse.move(startX, startY)
await page.mouse.down()
for (let i = 1; i <= 10; i++) {
await page.mouse.move(startX, startY + (targetY - startY) * (i / 10))
await page.waitForTimeout(25)
}
await page.mouse.up()
await page.waitForTimeout(500)

// The order flipped — the dragged row is no longer first.
const after = await pinnedOrder(page)
expect(after, 'pinned order should have the same two ids').toHaveLength(2)
expect(after[0], 'the dragged top row should now be second').not.toBe(topId)
expect(after).toContain(topId)
})
})
43 changes: 42 additions & 1 deletion apps/desktop/src/app/chat/session-drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ import { openSessionTile, type TileDock } from '@/store/session-states'

import { requestComposerInsertRefs } from './composer/focus'
import { type SessionDragPayload, sessionInlineRef, sessionLabel } from './composer/inline-refs'
import {
$sidebarReorderHint,
reorderIds,
type ReorderZoneSnapshot,
resolveReorderTarget,
snapshotReorderZones
} from './sidebar/reorder-zones'

/** A chat surface's drag-start geometry: the anchor pane id it advertises
* (`data-session-anchor`) and the composer a link drop routes to
Expand Down Expand Up @@ -101,11 +108,16 @@ export function startSessionDrag(
let surfaces: SurfaceSnapshot[] = []
let composers: ZoneRect[] = []
let zoneHost = new Map<string, null | string>()
let reorderZones: ReorderZoneSnapshot[] = []

// Commit intent, updated per resolved move (the machinery flushes the final
// move before commit, so these always match the released-at position).
let split: { anchor: string; before?: null | string; pos: TileDock } | null = null
let link: null | string = null
// A sidebar reorder target — set when the pointer is over a registered
// reorder zone (e.g. Pinned) that owns this session. Mutually exclusive with
// split/link: dropping within the bar reorders, dropping on chat links.
let reorder: { before: null | string; ids: string[]; onReorder: (ids: string[]) => void } | null = null

// The drag SOURCE (sidebar row or tile tab). Captured synchronously — React
// clears `currentTarget` after the pointerdown handler returns, but this runs
Expand All @@ -125,6 +137,7 @@ export function startSessionDrag(
surfaces = snapshotSurfaces()
composers = [...document.querySelectorAll<HTMLElement>('[data-slot="composer-root"]')].map(snapRect)
zoneHost = new Map(zones.map(zone => [zone.id, chatZonePane(zone.id)]))
reorderZones = snapshotReorderZones()
source?.style.setProperty('opacity', '0.45')
// The same sentinel the zone overlay + chat surfaces key off — the
// whole drop language (sheets, pills, caret, link overlay) lights up.
Expand All @@ -135,9 +148,30 @@ export function startSessionDrag(
if (source) {
source.style.opacity = restoreOpacity
}

$sidebarReorderHint.set(null)
},

resolveMove(x, y): DropHint | null {
// Sidebar reorder wins inside its own bounds: dropping a row within the
// bar reorders, only a drop over a chat surface links/splits. Checked
// first so the two never fight over the same pixels.
const reorderTarget = resolveReorderTarget(reorderZones, payload.id, x, y)

if (reorderTarget) {
reorder = reorderTarget
split = null
link = null
$sidebarReorderHint.set({ before: reorderTarget.before, draggedId: payload.id })

// Not a pane/zone drop — the tree overlay stands down. The pinned list
// paints its own insertion line off $sidebarReorderHint.
return null
}

reorder = null
$sidebarReorderHint.set(null)

const zone = zones.find(z => rectContains(z.rect, x, y))
const host = zone ? zoneHost.get(zone.id) : null

Expand Down Expand Up @@ -178,7 +212,14 @@ export function startSessionDrag(
},

onCommit() {
if (split) {
if (reorder) {
// Drop within the sidebar: reorder the list in place. No tile, no link.
const next = reorderIds(reorder.ids, payload.id, reorder.before)

if (next !== reorder.ids) {
reorder.onReorder(next)
}
} else if (split) {
openSessionTile(payload.id, split.pos, split.anchor, split.before)
// A tile for this session may already exist (openSessionTile is
// idempotent — e.g. persisted from an earlier run): a drop must never
Expand Down
4 changes: 2 additions & 2 deletions apps/desktop/src/app/chat/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ export function ChatSidebar({
{!trimmedQuery && (
<SidebarSessionsSection
activeSessionId={activeSidebarSessionId}
contentClassName={cn('flex max-h-44 flex-col gap-px rounded-lg pb-2 pt-1', GROUP_BODY)}
contentClassName={cn('flex max-h-[40vh] flex-col gap-px rounded-lg pb-2 pt-1', GROUP_BODY)}
dndSensors={dndSensors}
emptyState={<SidebarPinnedEmptyState />}
label={s.pinned}
Expand All @@ -1252,10 +1252,10 @@ export function ChatSidebar({
onTogglePin={unpinSession}
open={pinsOpen}
pinned
reorderViaDrag={pinnedSessions.length > 1}
rootClassName="shrink-0 p-0 pb-1"
sessions={pinnedSessions}
showProfileTags={showAllProfiles}
sortable={pinnedSessions.length > 1}
workingSessionIdSet={workingSessionIdSet}
/>
)}
Expand Down
58 changes: 58 additions & 0 deletions apps/desktop/src/app/chat/sidebar/reorder-zone-list.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { act, cleanup, render } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'

import { ReorderZoneList } from './reorder-zone-list'
import { $sidebarReorderHint, snapshotReorderZones } from './reorder-zones'

afterEach(() => {
cleanup()
$sidebarReorderHint.set(null)
})

function items(ids: string[]) {
return ids.map(id => ({ id, node: <div data-testid={`row-${id}`}>{id}</div> }))
}

describe('ReorderZoneList', () => {
it('registers a reorder zone while mounted and unregisters on unmount', () => {
const { unmount } = render(<ReorderZoneList items={items(['a', 'b', 'c'])} onReorder={() => undefined} />)

// The drag resolver discovers the zone via snapshotReorderZones().
const snaps = snapshotReorderZones()
expect(snaps).toHaveLength(1)
expect(snaps[0]!.ids).toEqual(['a', 'b', 'c'])

unmount()
expect(snapshotReorderZones()).toHaveLength(0)
})

it('tags each row so the resolver can find it by id', () => {
render(<ReorderZoneList items={items(['a', 'b'])} onReorder={() => undefined} />)

expect(document.querySelector('[data-reorder-row="a"]')).toBeTruthy()

Check warning on line 32 in apps/desktop/src/app/chat/sidebar/reorder-zone-list.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / Typecheck & Test (apps/desktop)

Unexpected use of 'document'
expect(document.querySelector('[data-reorder-row="b"]')).toBeTruthy()

Check warning on line 33 in apps/desktop/src/app/chat/sidebar/reorder-zone-list.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / Typecheck & Test (apps/desktop)

Unexpected use of 'document'
})

it('exposes live ids to the registration (a reorder reads current order at engage)', () => {
const onReorder = vi.fn()
const { rerender } = render(<ReorderZoneList items={items(['a', 'b', 'c'])} onReorder={onReorder} />)

// Re-render with a new order (as a commit would produce) — the zone's
// getIds must report the latest, not the mount-time snapshot.
rerender(<ReorderZoneList items={items(['c', 'a', 'b'])} onReorder={onReorder} />)

expect(snapshotReorderZones()[0]!.ids).toEqual(['c', 'a', 'b'])
})

it('paints the insertion line only for a drag of one of its own rows', () => {
render(<ReorderZoneList items={items(['a', 'b', 'c'])} onReorder={() => undefined} />)

// A foreign drag (id not in this list) shows no line.
act(() => $sidebarReorderHint.set({ before: 'b', draggedId: 'foreign' }))
expect(document.querySelector('[data-reorder-line]')).toBeNull()

Check warning on line 52 in apps/desktop/src/app/chat/sidebar/reorder-zone-list.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / Typecheck & Test (apps/desktop)

Unexpected use of 'document'

// A drag of one of ours renders exactly one insertion line.
act(() => $sidebarReorderHint.set({ before: 'b', draggedId: 'a' }))
expect(document.querySelectorAll('[data-reorder-line]')).toHaveLength(1)

Check warning on line 56 in apps/desktop/src/app/chat/sidebar/reorder-zone-list.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / Typecheck & Test (apps/desktop)

Unexpected use of 'document'
})
})
73 changes: 73 additions & 0 deletions apps/desktop/src/app/chat/sidebar/reorder-zone-list.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { useStore } from '@nanostores/react'
import { Fragment, useEffect, useRef } from 'react'

import { $sidebarReorderHint, registerReorderZone } from './reorder-zones'

/**
* A pointer-drag reorderable list for a flat sidebar section (Pinned).
*
* Unlike the dnd-kit `ReorderableList` (which owns its own DndContext and a
* grab handle), this list has NO handle and NO second drag system: the rows are
* already session-drag sources (session-drag.ts), and that shared pointer drag
* now resolves a reorder when the drop lands inside a registered zone. This
* component is only the registration + the insertion-line UI — dropping a row
* within the bar reorders, dropping it on the chat links, one gesture routed by
* where it's released.
*
* Items are passed as `{ id, node }` so the list can tag each row for the
* resolver's geometry snapshot (`data-reorder-row`) and interleave the
* insertion line at the live hint without reaching into row markup. The zone
* container is `display: contents` so the row wrappers remain the direct flex
* children of the section body (unchanged gap/scroll); each wrapper is a real
* box so its rect reflects the row for slot math.
*/
export function ReorderZoneList({
items,
onReorder
}: {
items: { id: string; node: React.ReactNode }[]
onReorder: (ids: string[]) => void
}) {
const containerRef = useRef<HTMLDivElement | null>(null)
const hint = useStore($sidebarReorderHint)

// Keep the latest ids/onReorder readable by the (stable) registration without
// re-registering on every order change — the drag reads them live at engage.
const idsRef = useRef<string[]>([])
idsRef.current = items.map(item => item.id)
const onReorderRef = useRef(onReorder)
onReorderRef.current = onReorder

useEffect(() => {
const el = containerRef.current

if (!el) {
return
}

return registerReorderZone({
el,
getIds: () => idsRef.current,
onReorder: ids => onReorderRef.current(ids)
})
}, [])

// Only paint the insertion line for a drag that started in THIS list (its id
// is one of ours) — a chat/tile drag never shows a reorder caret here.
const showHint = hint !== null && idsRef.current.includes(hint.draggedId)
const lineBefore = showHint ? hint.before : undefined

const line = <div aria-hidden className="mx-1 my-px h-0.5 rounded-full bg-primary/70" data-reorder-line />

return (
<div className="contents" ref={containerRef}>
{items.map(item => (
<Fragment key={item.id}>
{showHint && lineBefore === item.id ? line : null}
<div data-reorder-row={item.id}>{item.node}</div>
</Fragment>
))}
{showHint && lineBefore === null ? line : null}
</div>
)
}
Loading
Loading