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
52 changes: 52 additions & 0 deletions packages/worker/client/routes/record-table-search-sync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
export type RecordTableSearchSync = {
lastExternalValue: string
pendingExternalValue: string | null
}

/**
* The reader just typed this string, so the coming URL update is theirs.
* Record it as the last applied value and drop any deferred external string
* so blur cannot write stale text back into the field.
*/
export function acknowledgeRecordTableSearchInput(
value: string,
): RecordTableSearchSync {
return { lastExternalValue: value, pendingExternalValue: null }
}

/**
* Reconcile an incoming `value` prop (URL / back-button) with the last
* applied or typed string. Focused changes wait for blur; a return to the
* last applied value clears a stale pending string instead of keeping it.
*/
export function reconcileRecordTableSearchExternalValue(
state: RecordTableSearchSync,
nextValue: string,
focused: boolean,
): { state: RecordTableSearchSync; applyValue: string | null } {
if (nextValue === state.lastExternalValue) {
return {
state: {
lastExternalValue: state.lastExternalValue,
pendingExternalValue: null,
},
applyValue: null,
}
}
if (focused) {
return {
state: {
lastExternalValue: state.lastExternalValue,
pendingExternalValue: nextValue,
},
applyValue: null,
}
}
return {
state: {
lastExternalValue: nextValue,
pendingExternalValue: null,
},
applyValue: nextValue,
}
}
66 changes: 66 additions & 0 deletions packages/worker/client/routes/record-table.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@ import { renderToString } from 'remix/ui/server'
import { expect, test } from 'vitest'
import {
RecordTable,
RecordTableSearch,
type RecordTableColumn,
} from '#client/routes/record-table.tsx'
import {
acknowledgeRecordTableSearchInput,
reconcileRecordTableSearchExternalValue,
} from '#client/routes/record-table-search-sync.ts'

const columns: Array<RecordTableColumn> = [
{ key: 'name', label: 'Name', primary: true },
Expand Down Expand Up @@ -133,6 +138,67 @@ test('record table keeps container drops, row links, and expand/pane selection c
expect(overflowHtml).toContain('overflow: clip')
})

test('record table search stays uncontrolled so the first keystroke cannot remount it', async () => {
const html = await renderToString(
jsx(RecordTableSearch, {
label: 'Search packages',
placeholder: 'Search by name, id, description, or tag',
value: '',
onInput() {},
}),
)

expect(html).toContain('type="search"')
expect(html).toContain('aria-label="Search packages"')
// Passing `value` makes Remix restore the previous query on `input` and
// drop focus. `defaultValue` serializes as the HTML value attribute, so
// the client contract is "no controlled value prop" — pinned here by the
// WebKit cancel-button rules that otherwise appear on the first character.
expect(html).toContain('::-webkit-search-cancel-button')
expect(html).toContain('display: none')
})

test('record table search defers focused URL updates and drops a stale pending string', () => {
const empty = { lastExternalValue: '', pendingExternalValue: null }

// A keystroke is user-driven: the coming URL update must not become
// pending, or blur would overwrite whatever the reader typed next.
const typed = acknowledgeRecordTableSearchInput('ab')
expect(reconcileRecordTableSearchExternalValue(typed, 'ab', true)).toEqual({
state: { lastExternalValue: 'ab', pendingExternalValue: null },
applyValue: null,
})

// Clearing the field returns `q` to the last applied empty string.
// Without acknowledging the keystroke, pending would stay "ab" and
// blur would write that stale text back.
const cleared = acknowledgeRecordTableSearchInput('')
expect(reconcileRecordTableSearchExternalValue(cleared, '', true)).toEqual({
state: empty,
applyValue: null,
})
expect(
reconcileRecordTableSearchExternalValue(
{ lastExternalValue: '', pendingExternalValue: 'ab' },
'',
true,
),
).toEqual({
state: empty,
applyValue: null,
})

// Back-button while focused defers until blur; unfocused applies now.
expect(reconcileRecordTableSearchExternalValue(typed, '', true)).toEqual({
state: { lastExternalValue: 'ab', pendingExternalValue: '' },
applyValue: null,
})
expect(reconcileRecordTableSearchExternalValue(typed, '', false)).toEqual({
state: empty,
applyValue: '',
})
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

test('record table empty and busy states keep toolbar layout stable', async () => {
const emptyHtml = await renderToString(
jsx(RecordTable, {
Expand Down
116 changes: 95 additions & 21 deletions packages/worker/client/routes/record-table.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { css, type Handle } from 'remix/ui'
import { css, ref, type Handle } from 'remix/ui'
import { shouldRouterHandleClick } from '#client/client-router.tsx'
import { on } from '#client/event-mixin.ts'
import {
Expand All @@ -14,6 +14,11 @@ import {
getSurfaceCardCss,
hoverMq,
} from '#universal/styles/style-primitives.ts'
import {
acknowledgeRecordTableSearchInput,
reconcileRecordTableSearchExternalValue,
type RecordTableSearchSync,
} from './record-table-search-sync.ts'

/*
* The account and admin list/detail screens, as one table.
Expand Down Expand Up @@ -315,6 +320,37 @@ const footerCss = {
* they cost 48px, so the label becomes the control's accessible name.
*/

/**
* Live filter fields write the query into the URL on every keystroke. A
* Remix-controlled `value` lets the first character schedule a restore of the
* previous (empty) query, and WebKit's search cancel control appears on that
* same keystroke — either one drops focus and the reader has to click back in.
* The field stays uncontrolled. URL/back-button updates apply immediately
* when it is not focused, and on blur if they arrived while it was. A
* keystroke records the typed string as the last applied value so a later
* render that returns `q` to that value (clear, or back to the same query)
* cannot leave a stale pending string for blur to write back.
*/
const searchCancelHiddenCss = {
'&::-webkit-search-decoration': {
WebkitAppearance: 'none',
appearance: 'none',
},
'&::-webkit-search-cancel-button': {
WebkitAppearance: 'none',
appearance: 'none',
display: 'none',
},
'&::-webkit-search-results-button': {
WebkitAppearance: 'none',
appearance: 'none',
},
'&::-webkit-search-results-decoration': {
WebkitAppearance: 'none',
appearance: 'none',
},
} as const

export function RecordTableSearch(
handle: Handle<{
label: string
Expand All @@ -323,26 +359,64 @@ export function RecordTableSearch(
onInput: (value: string) => void
}>,
) {
return () => (
<input
type="search"
data-field-ring
value={handle.props.value}
placeholder={handle.props.placeholder}
aria-label={handle.props.label}
mix={[
on('input', (event) =>
handle.props.onInput((event.currentTarget as HTMLInputElement).value),
),
css({
...getAuthInputCss(),
flex: '1 1 12rem',
minWidth: '7rem',
width: 'auto',
}),
]}
/>
)
let input: HTMLInputElement | null = null
const initialValue = handle.props.value
let sync: RecordTableSearchSync = {
lastExternalValue: initialValue,
pendingExternalValue: null,
}

function applyExternalValue(nextValue: string) {
if (input) input.value = nextValue
sync = acknowledgeRecordTableSearchInput(nextValue)
}

return () => {
const nextValue = handle.props.value
const focused = Boolean(input && document.activeElement === input)
const reconciled = reconcileRecordTableSearchExternalValue(
sync,
nextValue,
focused,
)
sync = reconciled.state
if (reconciled.applyValue !== null)
applyExternalValue(reconciled.applyValue)
return (
<input
type="search"
data-field-ring
defaultValue={initialValue}
placeholder={handle.props.placeholder}
aria-label={handle.props.label}
mix={[
ref((node, signal) => {
input = node as HTMLInputElement
signal.addEventListener('abort', () => {
if (input === node) input = null
})
}),
on('input', (event) => {
const value = (event.currentTarget as HTMLInputElement).value
sync = acknowledgeRecordTableSearchInput(value)
handle.props.onInput(value)
}),
on('blur', () => {
if (sync.pendingExternalValue !== null) {
applyExternalValue(sync.pendingExternalValue)
}
Comment thread
cursor[bot] marked this conversation as resolved.
}),
css({
...getAuthInputCss(),
flex: '1 1 12rem',
minWidth: '7rem',
width: 'auto',
...searchCancelHiddenCss,
}),
]}
/>
)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

export function RecordTableSelect(
Expand Down
Loading