fix(web): address trajectory pagination review
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
- row "Load earlier history":
|
- row "Load earlier history":
|
||||||
- cell "Load earlier history":
|
- cell "Load earlier history":
|
||||||
- button "Load earlier history"
|
- button "Load earlier history":
|
||||||
|
- status
|
||||||
|
|||||||
@@ -157,10 +157,16 @@ async function loadToFirstTurn(page: Page): Promise<void> {
|
|||||||
await scrollToRatio(page, 0)
|
await scrollToRatio(page, 0)
|
||||||
if (await page.getByText(marker, { exact: false }).count() > 0) return
|
if (await page.getByText(marker, { exact: false }).count() > 0) return
|
||||||
const before = await logicalRows(page)
|
const before = await logicalRows(page)
|
||||||
|
const anchor = await firstVisibleRow(page)
|
||||||
await expect.poll(async () => ({
|
await expect.poll(async () => ({
|
||||||
marker: await page.getByText(marker, { exact: false }).count() > 0,
|
marker: await page.getByText(marker, { exact: false }).count() > 0,
|
||||||
rows: await logicalRows(page),
|
rows: await logicalRows(page),
|
||||||
}), { timeout: 30_000 }).not.toEqual({ marker: false, rows: before })
|
}), { timeout: 30_000 }).not.toEqual({ marker: false, rows: before })
|
||||||
|
await nextPaint(page)
|
||||||
|
await expect.poll(async () => {
|
||||||
|
const top = await rowTop(page, anchor.key)
|
||||||
|
return top === null ? Number.POSITIVE_INFINITY : Math.abs(top - anchor.top)
|
||||||
|
}, { timeout: 15_000 }).toBeLessThanOrEqual(GEOMETRY_TOLERANCE)
|
||||||
}
|
}
|
||||||
throw new Error('trajectory did not reach the first turn after twelve older-page requests')
|
throw new Error('trajectory did not reach the first turn after twelve older-page requests')
|
||||||
}
|
}
|
||||||
@@ -246,11 +252,12 @@ describe('web e2e: Trajectory virtualization over tail-paged history', () => {
|
|||||||
scaffold.workspaceCwd,
|
scaffold.workspaceCwd,
|
||||||
)
|
)
|
||||||
await compareOrRefreshGolden(LOAD_MORE_EXPECTED, loadMoreSnapshot, MODE)
|
await compareOrRefreshGolden(LOAD_MORE_EXPECTED, loadMoreSnapshot, MODE)
|
||||||
|
// Avoid Playwright scrolling the offscreen first row into the automatic-load threshold.
|
||||||
await loadMore.evaluate((button: HTMLButtonElement) => { button.click() })
|
await loadMore.evaluate((button: HTMLButtonElement) => { button.click() })
|
||||||
await expect.poll(() => held, { timeout: 15_000 }).toBe(true)
|
await expect.poll(() => held, { timeout: 15_000 }).toBe(true)
|
||||||
await expect.poll(async () => ({
|
await expect.poll(async () => ({
|
||||||
disabled: await loadMore.isDisabled(),
|
disabled: await loadMore.isDisabled(),
|
||||||
label: await loadMore.textContent(),
|
label: await loadMore.getAttribute('aria-label'),
|
||||||
}), { timeout: 15_000 }).toEqual({
|
}), { timeout: 15_000 }).toEqual({
|
||||||
disabled: true,
|
disabled: true,
|
||||||
label: 'Loading earlier history…',
|
label: 'Loading earlier history…',
|
||||||
|
|||||||
@@ -93,6 +93,15 @@
|
|||||||
cursor: default;
|
cursor: default;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.visuallyHidden {
|
||||||
|
position: absolute;
|
||||||
|
width: 1px;
|
||||||
|
height: 1px;
|
||||||
|
overflow: hidden;
|
||||||
|
clip: rect(0 0 0 0);
|
||||||
|
white-space: nowrap;
|
||||||
|
}
|
||||||
|
|
||||||
.table:not([data-scroll-ready='true']) {
|
.table:not([data-scroll-ready='true']) {
|
||||||
visibility: hidden;
|
visibility: hidden;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ import css from './TrajectoryTable.module.css'
|
|||||||
|
|
||||||
const BOTTOM_FOLLOW_THRESHOLD_PX = 2
|
const BOTTOM_FOLLOW_THRESHOLD_PX = 2
|
||||||
const OLDER_LOAD_THRESHOLD_PX = 48
|
const OLDER_LOAD_THRESHOLD_PX = 48
|
||||||
|
const HISTORY_LOAD_ROW_HEIGHT_PX = 30
|
||||||
const VIRTUALIZATION_THRESHOLD = 100
|
const VIRTUALIZATION_THRESHOLD = 100
|
||||||
const VIRTUAL_OVERSCAN_ROWS = 12
|
const VIRTUAL_OVERSCAN_ROWS = 12
|
||||||
const VIRTUAL_INITIAL_VIEWPORT_HEIGHT_PX = 600
|
const VIRTUAL_INITIAL_VIEWPORT_HEIGHT_PX = 600
|
||||||
@@ -364,7 +365,7 @@ export interface TrajectoryTableProps {
|
|||||||
recordFocus?: { readonly index: number } | null
|
recordFocus?: { readonly index: number } | null
|
||||||
/** Whether the initial history tail is still loading. */
|
/** Whether the initial history tail is still loading. */
|
||||||
historyLoading?: boolean
|
historyLoading?: boolean
|
||||||
/** Whether another surface is loading one older history page. */
|
/** Whether one older history page request is pending anywhere. */
|
||||||
olderHistoryLoading?: boolean
|
olderHistoryLoading?: boolean
|
||||||
/** First loaded raw event, used to preserve scroll position after prepending a page. */
|
/** First loaded raw event, used to preserve scroll position after prepending a page. */
|
||||||
historyStartSeq?: number | undefined
|
historyStartSeq?: number | undefined
|
||||||
@@ -1772,6 +1773,7 @@ export function TrajectoryTable({
|
|||||||
const virtualRowStructure = useStableVirtualRowStructure(projectedVirtualRows)
|
const virtualRowStructure = useStableVirtualRowStructure(projectedVirtualRows)
|
||||||
const virtualizationEnabled = hasOlderRecords
|
const virtualizationEnabled = hasOlderRecords
|
||||||
|| records.length > VIRTUALIZATION_THRESHOLD
|
|| records.length > VIRTUALIZATION_THRESHOLD
|
||||||
|
const virtualScrollMargin = hasOlderRecords ? HISTORY_LOAD_ROW_HEIGHT_PX : 0
|
||||||
const estimateVirtualRowSize = useCallback(
|
const estimateVirtualRowSize = useCallback(
|
||||||
(index: number) => virtualRowStructure[index]?.height ?? 30,
|
(index: number) => virtualRowStructure[index]?.height ?? 30,
|
||||||
[virtualRowStructure],
|
[virtualRowStructure],
|
||||||
@@ -1790,6 +1792,7 @@ export function TrajectoryTable({
|
|||||||
initialRect: { width: 0, height: VIRTUAL_INITIAL_VIEWPORT_HEIGHT_PX },
|
initialRect: { width: 0, height: VIRTUAL_INITIAL_VIEWPORT_HEIGHT_PX },
|
||||||
anchorTo: 'end',
|
anchorTo: 'end',
|
||||||
overscan: VIRTUAL_OVERSCAN_ROWS,
|
overscan: VIRTUAL_OVERSCAN_ROWS,
|
||||||
|
scrollMargin: virtualScrollMargin,
|
||||||
scrollEndThreshold: BOTTOM_FOLLOW_THRESHOLD_PX,
|
scrollEndThreshold: BOTTOM_FOLLOW_THRESHOLD_PX,
|
||||||
})
|
})
|
||||||
const virtualIndexByRecordId = useMemo(() => {
|
const virtualIndexByRecordId = useMemo(() => {
|
||||||
@@ -1804,10 +1807,15 @@ export function TrajectoryTable({
|
|||||||
return indexes
|
return indexes
|
||||||
}, [projectedVirtualRows])
|
}, [projectedVirtualRows])
|
||||||
const virtualItems = virtualizationEnabled ? rowVirtualizer.getVirtualItems() : []
|
const virtualItems = virtualizationEnabled ? rowVirtualizer.getVirtualItems() : []
|
||||||
const virtualTop = virtualItems[0]?.start ?? 0
|
const virtualTop = Math.max(0, (virtualItems[0]?.start ?? 0) - virtualScrollMargin)
|
||||||
const virtualBottom = virtualItems.length === 0
|
const virtualBottom = virtualItems.length === 0
|
||||||
? 0
|
? 0
|
||||||
: Math.max(0, rowVirtualizer.getTotalSize() - (virtualItems.at(-1)?.end ?? 0))
|
: Math.max(
|
||||||
|
0,
|
||||||
|
rowVirtualizer.getTotalSize()
|
||||||
|
+ virtualScrollMargin
|
||||||
|
- (virtualItems.at(-1)?.end ?? 0),
|
||||||
|
)
|
||||||
const renderedRecords = virtualizationEnabled
|
const renderedRecords = virtualizationEnabled
|
||||||
? virtualItems.flatMap((item) => {
|
? virtualItems.flatMap((item) => {
|
||||||
const row = projectedVirtualRows[item.index]
|
const row = projectedVirtualRows[item.index]
|
||||||
@@ -2220,12 +2228,19 @@ export function TrajectoryTable({
|
|||||||
</colgroup>
|
</colgroup>
|
||||||
<tbody>
|
<tbody>
|
||||||
{hasOlderRecords && (
|
{hasOlderRecords && (
|
||||||
<tr className={css.historyLoadRow} data-history-load="">
|
<tr
|
||||||
|
className={css.historyLoadRow}
|
||||||
|
data-history-load=""
|
||||||
|
aria-rowindex={1}
|
||||||
|
>
|
||||||
<td colSpan={2}>
|
<td colSpan={2}>
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
className={css.historyLoadButton}
|
className={css.historyLoadButton}
|
||||||
disabled={olderBusy || onLoadOlder === undefined}
|
disabled={olderBusy || onLoadOlder === undefined}
|
||||||
|
aria-label={olderBusy
|
||||||
|
? 'Loading earlier history…'
|
||||||
|
: 'Load earlier history'}
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
const pane = tablePaneRef.current
|
const pane = tablePaneRef.current
|
||||||
if (pane !== null) requestOlder(pane, false)
|
if (pane !== null) requestOlder(pane, false)
|
||||||
@@ -2234,12 +2249,12 @@ export function TrajectoryTable({
|
|||||||
{olderBusy && (
|
{olderBusy && (
|
||||||
<span className={css.historyLoadingSpinner} aria-hidden="true" />
|
<span className={css.historyLoadingSpinner} aria-hidden="true" />
|
||||||
)}
|
)}
|
||||||
<span
|
<span aria-hidden="true">
|
||||||
role={olderBusy ? 'status' : undefined}
|
|
||||||
aria-live={olderBusy ? 'polite' : undefined}
|
|
||||||
>
|
|
||||||
{olderBusy ? 'Loading earlier history…' : 'Load earlier history'}
|
{olderBusy ? 'Loading earlier history…' : 'Load earlier history'}
|
||||||
</span>
|
</span>
|
||||||
|
<span className={css.visuallyHidden} role="status" aria-live="polite">
|
||||||
|
{olderBusy ? 'Loading earlier history…' : ''}
|
||||||
|
</span>
|
||||||
</button>
|
</button>
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
|
|||||||
@@ -418,7 +418,9 @@ describe('TrajectoryTable', () => {
|
|||||||
await waitFor(() => { expect(onLoadOlder).toHaveBeenCalledOnce() })
|
await waitFor(() => { expect(onLoadOlder).toHaveBeenCalledOnce() })
|
||||||
expect(screen.getByRole('status').textContent).toContain('Loading earlier history…')
|
expect(screen.getByRole('status').textContent).toContain('Loading earlier history…')
|
||||||
resolveOlder?.(true)
|
resolveOlder?.(true)
|
||||||
await waitFor(() => { expect(screen.queryByRole('status')).toBeNull() })
|
await waitFor(() => {
|
||||||
|
expect(screen.getByRole('status').textContent).toBe('')
|
||||||
|
})
|
||||||
scrollHeight = 260
|
scrollHeight = 260
|
||||||
view.rerender(
|
view.rerender(
|
||||||
<TrajectoryTable
|
<TrajectoryTable
|
||||||
@@ -458,7 +460,10 @@ describe('TrajectoryTable', () => {
|
|||||||
|
|
||||||
const table = screen.getByRole('table')
|
const table = screen.getByRole('table')
|
||||||
const loadButton = screen.getByRole('button', { name: 'Load earlier history' })
|
const loadButton = screen.getByRole('button', { name: 'Load earlier history' })
|
||||||
expect(table.querySelector('tbody > tr:first-child')?.contains(loadButton)).toBe(true)
|
const loadRow = table.querySelector('tbody > tr:first-child')
|
||||||
|
expect(loadRow?.contains(loadButton)).toBe(true)
|
||||||
|
expect(loadRow?.getAttribute('aria-rowindex')).toBe('1')
|
||||||
|
expect(screen.getByRole('status').textContent).toBe('')
|
||||||
expect(table.getAttribute('aria-rowcount')).toBe('4')
|
expect(table.getAttribute('aria-rowcount')).toBe('4')
|
||||||
expect((await screen.findByRole('row', { name: /ASSISTANT/ })).getAttribute('aria-rowindex'))
|
expect((await screen.findByRole('row', { name: /ASSISTANT/ })).getAttribute('aria-rowindex'))
|
||||||
.toBe('2')
|
.toBe('2')
|
||||||
|
|||||||
Reference in New Issue
Block a user