From f219ba22a61e68fa1bcda3dfeb921955aba36aa4 Mon Sep 17 00:00:00 2001 From: Luca Martial <48870843+luca-martial@users.noreply.github.com> Date: Wed, 13 May 2026 17:28:48 -0400 Subject: [PATCH] feat(cli): redesign Notion page picker UI and add skip-empty flow (#78) Rework the inline picker to use a cleaner visual style (filled/empty square glyphs, bordered layout, clack-style header) and streamlined keybindings (Enter to confirm, Escape to quit, Right Arrow to expand). Replace the transient "select at least one" hint with a skip-empty confirmation prompt that exits cleanly via quit-without-save. Co-authored-by: Claude Opus 4.6 (1M context) --- .../cli/src/notion-page-picker-tree.test.ts | 25 ++--- packages/cli/src/notion-page-picker-tree.ts | 15 +-- .../cli/src/notion-page-picker-tui.test.tsx | 70 ++++++------ packages/cli/src/notion-page-picker-tui.tsx | 102 ++++++++++++------ 4 files changed, 126 insertions(+), 86 deletions(-) diff --git a/packages/cli/src/notion-page-picker-tree.test.ts b/packages/cli/src/notion-page-picker-tree.test.ts index 94b46b57..58e8c7ca 100644 --- a/packages/cli/src/notion-page-picker-tree.test.ts +++ b/packages/cli/src/notion-page-picker-tree.test.ts @@ -11,7 +11,6 @@ import { selectAllVisible, selectNone, toggleChecked, - TRANSIENT_HINT_DURATION_MS, visibleNodeIds, type NotionPickerPageInput, } from './notion-page-picker-tree.js'; @@ -223,22 +222,24 @@ describe('bulk actions and reducer effects', () => { }); }); - it('blocks empty saves, updates search state, and quits without saving', () => { + it('prompts skip-empty confirmation on empty save, updates search state, and quits without saving', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), existingRootPageIds: [], currentCrawlMode: 'selected_roots', }); - const blockedSave = reducer(state, 'save-request', 9000); - expect(blockedSave).toEqual({ - next: { - ...state, - transientHint: { - text: 'Select at least one page or press q to quit', - expiresAt: 9000 + TRANSIENT_HINT_DURATION_MS, - }, - }, + const emptySave = reducer(state, 'save-request'); + expect(emptySave).toEqual({ + next: { ...state, pendingConfirm: 'skip-empty' }, + effect: null, + }); + expect(reducer(emptySave.next, 'save-confirm')).toEqual({ + next: { ...state, pendingConfirm: null }, + effect: 'quit-without-save', + }); + expect(reducer(emptySave.next, 'save-cancel')).toEqual({ + next: { ...state, pendingConfirm: null }, effect: null, }); expect( @@ -262,7 +263,7 @@ describe('bulk actions and reducer effects', () => { const withHint = { ...state, transientHint: { - text: 'Select at least one page or press q to quit', + text: 'Select at least one page or press esc to cancel', expiresAt: 11500, }, }; diff --git a/packages/cli/src/notion-page-picker-tree.ts b/packages/cli/src/notion-page-picker-tree.ts index 379ac938..738ab723 100644 --- a/packages/cli/src/notion-page-picker-tree.ts +++ b/packages/cli/src/notion-page-picker-tree.ts @@ -22,7 +22,7 @@ export interface PickerState { checked: Set; cursorId: string; search: { editing: boolean; query: string }; - pendingConfirm: 'mode-switch' | null; + pendingConfirm: 'mode-switch' | 'skip-empty' | null; preLoadWarnings: string[]; transientHint: { text: string; expiresAt: number } | null; currentCrawlMode: 'all_accessible' | 'selected_roots'; @@ -61,7 +61,7 @@ interface MutableNode { childIds: string[]; } -export const TRANSIENT_HINT_DURATION_MS = 2500; +const TRANSIENT_HINT_DURATION_MS = 2500; const collator = new Intl.Collator('en', { sensitivity: 'base', numeric: true }); @@ -444,7 +444,8 @@ export function buildInitialState(args: { export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now()): { next: PickerState; effect: PickerEffect } { if (state.pendingConfirm) { if (cmd === 'save-confirm') { - return { next: cloneState(state, { pendingConfirm: null }), effect: 'save' }; + const effect: PickerEffect = state.pendingConfirm === 'skip-empty' ? 'quit-without-save' : 'save'; + return { next: cloneState(state, { pendingConfirm: null }), effect }; } if (cmd === 'save-cancel') { return { next: cloneState(state, { pendingConfirm: null }), effect: null }; @@ -498,19 +499,13 @@ export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now() }; case 'save-request': if (state.checked.size === 0) { - return { - next: cloneState(state, { - transientHint: transientHint('Select at least one page or press q to quit', now), - }), - effect: null, - }; + return { next: cloneState(state, { pendingConfirm: 'skip-empty' }), effect: null }; } if (state.currentCrawlMode === 'all_accessible') { return { next: cloneState(state, { pendingConfirm: 'mode-switch' }), effect: null }; } return { next: state, effect: 'save' }; case 'save-confirm': - return { next: state, effect: 'save' }; case 'save-cancel': return { next: state, effect: null }; case 'quit': diff --git a/packages/cli/src/notion-page-picker-tui.test.tsx b/packages/cli/src/notion-page-picker-tui.test.tsx index 2d4dffc3..16ad93db 100644 --- a/packages/cli/src/notion-page-picker-tui.test.tsx +++ b/packages/cli/src/notion-page-picker-tui.test.tsx @@ -1,7 +1,7 @@ /* @jsxImportSource react */ import { render as renderInkTest } from 'ink-testing-library'; -import { act, type ReactNode } from 'react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { type ReactNode } from 'react'; +import { describe, expect, it, vi } from 'vitest'; import { buildInitialState, buildPickerTree, type NotionPickerPageInput } from './notion-page-picker-tree.js'; import { NotionPickerApp, @@ -70,13 +70,9 @@ function fakeInkInstance(): NotionPickerInkInstance { } function normalizeFrameWrap(frame: string | undefined): string { - return frame?.replace(/\n/g, ' ') ?? ''; + return frame?.replace(/\n/g, ' ').replace(/│ /g, '').replace(/ +/g, ' ') ?? ''; } -afterEach(() => { - vi.useRealTimers(); -}); - describe('notionPickerCommandForInkInput', () => { it('maps browse, search, and confirm input to reducer commands', () => { expect(notionPickerCommandForInkInput('', { downArrow: true }, state().search, null)).toBe('cursor-down'); @@ -87,9 +83,11 @@ describe('notionPickerCommandForInkInput', () => { expect(notionPickerCommandForInkInput('/', {}, state().search, null)).toBe('search-start'); expect(notionPickerCommandForInkInput('a', {}, state().search, null)).toBe('select-all-visible'); expect(notionPickerCommandForInkInput('n', {}, state().search, null)).toBe('select-none'); - expect(notionPickerCommandForInkInput('s', {}, state().search, null)).toBe('save-request'); - expect(notionPickerCommandForInkInput('q', {}, state().search, null)).toBe('quit'); + expect(notionPickerCommandForInkInput('', { return: true }, state().search, null)).toBe('save-request'); + expect(notionPickerCommandForInkInput('', { escape: true }, state().search, null)).toBe('quit'); expect(notionPickerCommandForInkInput('c', { ctrl: true }, state().search, null)).toBe('quit'); + expect(notionPickerCommandForInkInput('s', {}, state().search, null)).toBeNull(); + expect(notionPickerCommandForInkInput('q', {}, state().search, null)).toBeNull(); expect(notionPickerCommandForInkInput('x', {}, { editing: true, query: '' }, null)).toEqual({ type: 'search-input', @@ -145,13 +143,16 @@ describe('NotionPickerApp', () => { ); const frame = lastFrame() ?? ''; - expect(frame).toContain('Notion pages visible to integration "Design Workspace"'); + expect(frame).toContain('Select Notion pages to ingest'); + expect(frame).toContain('Workspace: Design Workspace'); expect(frame).toContain('5000-page cap reached - some pages not shown'); expect(frame).toContain('1 stored root_page_ids no longer visible - they will be removed if you save'); - expect(frame).toContain('▸ [ ] Engineering Docs ▸ (1)'); - expect(frame).toContain(' [ ] Marketing'); + expect(frame).toContain('◻ Engineering Docs ▸ (1)'); + expect(frame).toContain('◻ Marketing'); expect(frame).not.toContain('Search ready: -'); - expect(frame).toContain('space toggle · enter expand · / search · a all · n none · s save & exit · q quit'); + expect(normalizeFrameWrap(frame)).toContain( + 'Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape to go back, or Ctrl+C to exit.', + ); }); it('renders partial discovery warnings without stale-root save suffix', () => { @@ -199,8 +200,8 @@ describe('NotionPickerApp', () => { ); const frame = lastFrame() ?? ''; - expect(frame).toContain('▸ [×] Engineering Docs ▾'); - expect(frame).toContain(' [~] Architecture'); + expect(frame).toContain('◼ Engineering Docs ▾'); + expect(frame).toContain(' ◼ Architecture'); }); it('supports keyboard selection, all_accessible confirmation, and save callback', async () => { @@ -220,12 +221,12 @@ describe('NotionPickerApp', () => { stdin.write(' '); await waitForInkInput(); - expect(lastFrame()).toContain('[×] Engineering Docs'); + expect(lastFrame()).toContain('◼ Engineering Docs'); - stdin.write('s'); + stdin.write('\r'); await waitForInkInput(); expect(normalizeFrameWrap(lastFrame())).toContain( - 'Save will switch crawl_mode all_accessible -> selected_roots and limit ingest to 1 selected page. [y] confirm [esc] back', + 'Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to 1 selected page. Press Enter to confirm or Escape to go back.', ); stdin.write('y'); @@ -233,8 +234,7 @@ describe('NotionPickerApp', () => { expect(onExit).toHaveBeenCalledWith({ kind: 'save', rootPageIds: [IDS.engineering] }); }); - it('removes transient hints after their expiry time', async () => { - vi.useFakeTimers(); + it('prompts skip-empty confirmation on empty submit and dismisses on cancel', async () => { const onExit = vi.fn(); const { stdin, lastFrame } = renderInkTest( { />, ); - await act(async () => { - stdin.write('s'); - await vi.advanceTimersByTimeAsync(10); - }); - expect(lastFrame()).toContain('Select at least one page or press q to quit'); - - await act(async () => { - await vi.advanceTimersByTimeAsync(2500); - }); - expect(lastFrame()).not.toContain('Select at least one page or press q to quit'); + stdin.write('\r'); + await waitForInkInput(); + expect(normalizeFrameWrap(lastFrame())).toContain( + 'Nothing selected. Skip this step? Press Enter to skip or Escape to go back.', + ); expect(onExit).not.toHaveBeenCalled(); + + stdin.write('n'); + await waitForInkInput(); + expect(lastFrame()).not.toContain('Nothing selected. Skip this step?'); + expect(onExit).not.toHaveBeenCalled(); + + stdin.write('\r'); + await waitForInkInput(); + expect(lastFrame()).toContain('Nothing selected. Skip this step?'); + + stdin.write('\r'); + await waitForInkInput(); + expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); }); it('renders row-window overflow indicators when the visible list is clipped', async () => { @@ -312,7 +320,7 @@ describe('NotionPickerApp', () => { />, ); - stdin.write('q'); + stdin.write('\u0003'); await waitForInkInput(); expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); }); diff --git a/packages/cli/src/notion-page-picker-tui.tsx b/packages/cli/src/notion-page-picker-tui.tsx index 30af7522..d627d200 100644 --- a/packages/cli/src/notion-page-picker-tui.tsx +++ b/packages/cli/src/notion-page-picker-tui.tsx @@ -16,6 +16,7 @@ const COLOR_THEME = { text: 'white', muted: 'gray', active: 'cyan', + selected: 'green', warning: 'yellow', } as const; @@ -23,6 +24,7 @@ const NO_COLOR_THEME = { text: 'white', muted: 'white', active: 'white', + selected: 'white', warning: 'white', } as const; @@ -158,13 +160,12 @@ export function notionPickerCommandForInkInput( if (key.downArrow) return 'cursor-down'; if (key.leftArrow) return 'cursor-left'; if (key.rightArrow) return 'cursor-right'; - if (key.return) return 'expand'; + if (key.return) return 'save-request'; if (input === ' ') return 'toggle-check'; if (input === '/') return 'search-start'; if (input === 'a') return 'select-all-visible'; if (input === 'n') return 'select-none'; - if (input === 's') return 'save-request'; - if (input === 'q' || key.escape) return 'quit'; + if (key.escape) return 'quit'; return null; } @@ -174,18 +175,27 @@ function PickerRow(props: { state: PickerState; nodeId: string; width: number; t const focused = props.state.cursorId === node.id; const locked = isAncestorChecked(node.id, props.state.checked, props.state.byId); const checked = props.state.checked.has(node.id); - const glyph = locked ? '[~]' : checked ? '[×]' : '[ ]'; - const children = + const isSelected = checked || locked; + const glyph = isSelected ? '◼' : '◻'; + const glyphColor = locked ? props.theme.muted : checked ? props.theme.selected : props.theme.muted; + const childAffordance = node.childIds.length > 0 ? (props.state.expanded.has(node.id) ? ' ▾' : ` ▸ (${node.childIds.length})`) : ''; - const prefix = `${focused ? '▸' : ' '} ${glyph} ${' '.repeat(node.depth * 2)}`; - const color = focused ? props.theme.active : locked || node.archived ? props.theme.muted : props.theme.text; - const title = truncateText(`${node.title}${children}`, Math.max(10, props.width - prefix.length)); + const indent = ' '.repeat(node.depth * 2); + const titleColor = focused ? props.theme.text : props.theme.muted; const inverse = rowMatchesSearch(props.state, node.id); + const prefixWidth = indent.length + 2; + const title = truncateText(`${node.title}${childAffordance}`, Math.max(10, props.width - prefixWidth)); return ( - - {prefix} - {title} + + + {indent} + {glyph} + + + {' '} + {title} + ); } @@ -198,7 +208,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { const visibleIds = visibleNodeIds(state); const selectedIndex = Math.max(0, visibleIds.indexOf(state.cursorId)); const reservedRows = state.pendingConfirm === 'mode-switch' ? 9 : 8; - const visibleRows = Math.max(5, Math.min(20, (props.terminalRows ?? 24) - reservedRows)); + const visibleRows = Math.max(5, Math.min(12, (props.terminalRows ?? 24) - reservedRows)); const rows = windowItems(visibleIds, selectedIndex, visibleRows); const hiddenAbove = rows.offset; const hiddenBelow = Math.max(0, visibleIds.length - rows.offset - rows.items.length); @@ -254,34 +264,60 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { return ( - Notion pages visible to integration "{props.workspaceLabel}" - {props.cappedAtCount ? {props.cappedAtCount}-page cap reached - some pages not shown : null} - {state.preLoadWarnings.map((warning) => ( - - {staleWarningText(warning)} - - ))} - {showSearch ? ( + + ◆ + Select Notion pages to ingest + + - / {state.search.query} - {state.search.editing ? '█' : ''} ({searchMatchCount} matches) + Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape + to go back, or Ctrl+C to exit. - ) : null} - + + Workspace: {props.workspaceLabel} + {props.cappedAtCount ? ( + {props.cappedAtCount}-page cap reached - some pages not shown + ) : null} + {state.preLoadWarnings.map((warning) => ( + + {staleWarningText(warning)} + + ))} + {showSearch ? ( + + / + + {state.search.query} + {state.search.editing ? '█' : ''} + + ({searchMatchCount} matches) + + ) : null} {hiddenAbove > 0 ? ↑ {hiddenAbove} more : null} {rows.items.map((nodeId) => ( ))} {hiddenBelow > 0 ? ↓ {hiddenBelow} more : null} + {state.pendingConfirm === 'mode-switch' ? ( + + Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to{' '} + {selectedPageCountText(selectedCount)}. Press Enter to confirm or Escape to go back. + + ) : null} + {state.pendingConfirm === 'skip-empty' ? ( + Nothing selected. Skip this step? Press Enter to skip or Escape to go back. + ) : null} + {state.transientHint ? {state.transientHint.text} : null} - {state.pendingConfirm === 'mode-switch' ? ( - - Save will switch crawl_mode all_accessible -> selected_roots and limit ingest to{' '} - {selectedPageCountText(selectedCount)}. [y] confirm [esc] back - - ) : null} - {state.transientHint ? {state.transientHint.text} : null} - space toggle · enter expand · / search · a all · n none · s save & exit · q quit + └ ); } @@ -323,7 +359,7 @@ export async function renderNotionPickerTui( exitOnCtrlC: false, patchConsole: false, maxFps: 30, - alternateScreen: true, + alternateScreen: false, }, ); await instance.waitUntilExit();