Skip to content

Commit cb0507a

Browse files
fix(connect): land the ↑/↓ snap on the edge you are travelling towards (#94)
Walking into an entry taller than the window put the highlight in the wrong place, so a long message was entered from the end you were leaving rather than the end you were heading for. Two bugs compounded: 1. snapToEntry inferred direction from the entry's POSITION rather than the direction of travel, so it top-aligned any too-tall entry. Backing UP into one from below jumped to its far-away first line instead of the last line just above you. It now takes a dir argument: down lands on the entry's first row, up on its last, and revealMore walks through the rest — the two read the entry continuously either way. 2. ensureVisible discarded the snap target entirely when the entry was the NEWEST one, pinning to the bottom to keep following streamed content. For a trailing entry taller than the window that meant walking down onto it showed its end. The bottom-follow is now keyed on the snap landing on the last screenful, which is what it always meant. So with two messages (short)(long), ↓ off (short) lands on the top of (long); with (short)(long)(short), ↑ off the trailing (short) lands on the bottom of (long). The bottom-follow decision moves into a pure snapAnchorForEntry so both halves are covered by tests. Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9d479f1 commit cb0507a

3 files changed

Lines changed: 100 additions & 25 deletions

File tree

src/ui/ConnectApp.tsx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import {
5353
padPanelBlocks,
5454
pendingMessageRows,
5555
rowViewport,
56-
snapToEntry,
56+
snapAnchorForEntry,
5757
spacerRow,
5858
type RowSpan,
5959
type ScrollAnchor,
@@ -988,18 +988,12 @@ export function ConnectApp(props: ConnectAppProps): React.ReactElement {
988988
[allRows, view],
989989
)
990990

991-
// Snap the window so a highlighted entry is readable: entering from above,
992-
// its first line goes to the top of the frame; from below, it aligns to the
993-
// bottom edge. See snapToEntry.
991+
// Snap the window so a highlighted entry is readable, given which way the
992+
// highlight is travelling (`dir`: 1 for ↓, -1 for ↑). See snapToEntry.
994993
const ensureVisible = useCallback(
995-
(key: string): void => {
996-
const target = snapToEntry(allRows, key, view, view.capacity)
997-
if (target === null) return
998-
const range = entryRange(allRows, key)
999-
// Snapping to the newest entry means following the bottom again, so new
1000-
// content keeps arriving in view.
1001-
if (range && range.last >= allRows.length - 1) setScrollAnchor(null)
1002-
else setScrollAnchor(anchorAt(allRows, target))
994+
(key: string, dir: 1 | -1 = 1): void => {
995+
const move = snapAnchorForEntry(allRows, key, view, view.capacity, dir)
996+
if (move) setScrollAnchor(move.anchor)
1003997
},
1004998
[allRows, view],
1005999
)
@@ -1107,7 +1101,7 @@ export function ConnectApp(props: ConnectAppProps): React.ReactElement {
11071101
const target = idx === -1 ? navKeys.length - 1 : Math.max(0, idx - 1)
11081102
if (navKeys.length > 0) {
11091103
setNavKey(navKeys[target])
1110-
ensureVisible(navKeys[target])
1104+
ensureVisible(navKeys[target], -1)
11111105
}
11121106
return
11131107
}
@@ -1118,7 +1112,7 @@ export function ConnectApp(props: ConnectAppProps): React.ReactElement {
11181112
setScrollAnchor(null)
11191113
} else {
11201114
setNavKey(navKeys[idx + 1])
1121-
ensureVisible(navKeys[idx + 1])
1115+
ensureVisible(navKeys[idx + 1], 1)
11221116
}
11231117
return
11241118
}

src/ui/transcriptRows.ts

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -519,27 +519,58 @@ export function entryRange(
519519
}
520520

521521
// Where the window must sit for `entryKey` to be readable, given where it sits
522-
// now — the ↑/↓ snap.
522+
// now and which way the highlight is travelling (`dir`: 1 for ↓, -1 for ↑) —
523+
// the ↑/↓ snap.
523524
//
524-
// An entry coming into frame from ABOVE puts its first line at the TOP of the
525-
// window: you read a message from its beginning, with as much of it in front
526-
// of you as fits. So does one too tall to fit whole. An entry arriving from
527-
// BELOW aligns to the bottom edge, the direction it was already travelling.
528-
// One already fully in frame doesn't move the window at all. Returns the flat
529-
// row index to pin to the top, or null to leave the window alone. Pure, for
530-
// tests.
525+
// An entry already fully in frame doesn't move the window at all. One that
526+
// FITS but sits off-frame comes in from the side it is on: from above to the
527+
// top of the window, from below to the bottom edge. One TOO TALL to fit lands
528+
// on the edge you are heading towards, so the walk keeps its direction of
529+
// travel: ↓ lands on its FIRST line (you read a long message from its
530+
// beginning) and ↑ on its LAST (you back into the end of it). From there
531+
// ↑/↓ scroll THROUGH the rest of it a row at a time — see revealMore — so the
532+
// two together read the entry continuously in whichever direction you started.
533+
//
534+
// Returns the flat row index to pin to the top, or null to leave the window
535+
// alone. Pure, for tests.
531536
export function snapToEntry(
532537
rows: readonly TranscriptRow[],
533538
entryKey: string,
534539
view: { start: number; end: number },
535540
capacity: number,
541+
dir: 1 | -1 = 1,
536542
): number | null {
537543
const range = entryRange(rows, entryKey)
538544
if (!range) return null
545+
// Already readable whole: don't jostle the window.
546+
if (range.first >= view.start && range.last < view.end) return null
547+
const bottomAligned = Math.max(0, range.last - capacity + 1)
539548
const height = range.last - range.first + 1
540-
if (range.first < view.start || height >= capacity) return range.first
541-
if (range.last >= view.end) return Math.max(0, range.last - capacity + 1)
542-
return null
549+
if (height >= capacity) return dir < 0 ? bottomAligned : range.first
550+
return range.first < view.start ? range.first : bottomAligned
551+
}
552+
553+
// snapToEntry's row index as the scroll move to apply: null to leave the window
554+
// where it is, or the anchor to park on — itself null when the snap lands on the
555+
// last screenful, which means following the bottom again so streamed content
556+
// keeps arriving in view.
557+
//
558+
// That bottom-follow is keyed on WHERE THE SNAP LANDS, not on the entry being
559+
// the newest one: an entry taller than the window is snapped to an interior row
560+
// (its first line, when walking down into it), and pinning to the bottom there
561+
// would throw the snap away and show the entry's end instead of its beginning.
562+
// Pure, for tests.
563+
export function snapAnchorForEntry(
564+
rows: readonly TranscriptRow[],
565+
entryKey: string,
566+
view: { start: number; end: number },
567+
capacity: number,
568+
dir: 1 | -1 = 1,
569+
): { anchor: ScrollAnchor | null } | null {
570+
const target = snapToEntry(rows, entryKey, view, capacity, dir)
571+
if (target === null) return null
572+
if (target >= rows.length - capacity) return { anchor: null }
573+
return { anchor: anchorAt(rows, target) }
543574
}
544575

545576
// Agent and user prose rendered as markdown (bold, headings, bullets, tables,

test/connect-app.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
itemRows,
2121
layOutItems,
2222
rowViewport,
23+
snapAnchorForEntry,
2324
snapToEntry,
2425
withRenderedMarkdown,
2526
type TranscriptRow,
@@ -902,6 +903,55 @@ describe('snapToEntry', () => {
902903
]
903904
expect(snapToEntry(short, 'b', { start: 0, end: 3 }, 3)).toBeNull()
904905
})
906+
907+
it('backs ↑ into a too-tall entry at its LAST line, keeping the walk going up', () => {
908+
// 'b' spans rows 1..10. Walking UP out of 'c' lands on 'b's bottom edge
909+
// (rows 7..10 on a 4-row window), not its far-away first line.
910+
expect(snapToEntry(rows, 'b', { start: 8, end: 12 }, 4, -1)).toBe(7)
911+
})
912+
913+
it('walks ↓ into a too-tall entry at its FIRST line', () => {
914+
expect(snapToEntry(rows, 'b', { start: 0, end: 4 }, 4, 1)).toBe(1)
915+
})
916+
})
917+
918+
describe('snapAnchorForEntry', () => {
919+
// A short entry, then one 10 rows tall — taller than the 4-row window.
920+
const shortThenLong: TranscriptRow[] = [
921+
{ id: 'a', entryKey: 'a', spans: [] },
922+
...Array.from({ length: 10 }, (_, i) => ({ id: `b${i}`, entryKey: 'b', spans: [] })),
923+
]
924+
925+
it('parks on the TOP of a trailing too-tall entry walked into from above', () => {
926+
// ↓ off the short entry onto the long last one: its first line, NOT the
927+
// bottom-follow that being the newest entry used to force.
928+
expect(snapAnchorForEntry(shortThenLong, 'b', { start: 0, end: 4 }, 4, 1)).toEqual({
929+
anchor: { entryKey: 'b', rowOffset: 0 },
930+
})
931+
})
932+
933+
it('follows the bottom when the snap really does land on the last screenful', () => {
934+
// The same trailing entry, but short enough to fit: bottom-aligning it IS
935+
// the bottom of the log, so keep streaming content in view.
936+
const shortTail: TranscriptRow[] = [
937+
...Array.from({ length: 6 }, (_, i) => ({ id: `a${i}`, entryKey: 'a', spans: [] })),
938+
{ id: 'b0', entryKey: 'b', spans: [] },
939+
]
940+
expect(snapAnchorForEntry(shortTail, 'b', { start: 0, end: 4 }, 4, 1)).toEqual({ anchor: null })
941+
})
942+
943+
it('parks on the BOTTOM of a too-tall entry walked into from below', () => {
944+
// (short) (long) (short) with the highlight on the trailing short one: ↑
945+
// lands on the long entry's END — rows 7..10 of an 11-row block.
946+
const withTail: TranscriptRow[] = [...shortThenLong, { id: 'c', entryKey: 'c', spans: [] }]
947+
expect(snapAnchorForEntry(withTail, 'b', { start: 8, end: 12 }, 4, -1)).toEqual({
948+
anchor: { entryKey: 'b', rowOffset: 6 },
949+
})
950+
})
951+
952+
it('reports no move for an entry already fully in frame', () => {
953+
expect(snapAnchorForEntry(shortThenLong, 'a', { start: 0, end: 4 }, 4, -1)).toBeNull()
954+
})
905955
})
906956

907957
describe('withRenderedMarkdown', () => {

0 commit comments

Comments
 (0)