mirror of
https://github.com/Kong/insomnia.git
synced 2026-10-06 21:14:38 -04:00
fix(key-value-editor): stop ListBox typeahead from swallowing typed spaces (#10563)
Background: in the environment variables table view and the generic key-value editor, a space typed within 1s of the previous printable character is swallowed (typing "My key" produces "Mykey"); users had to insert spaces via arrow keys afterwards. Raw JSON editing is unaffected. The react-aria ListBox typeahead (useTypeSelect) intercepts space keydowns in the capture phase while its search buffer is non-empty. Disable typeahead via disallowTypeAhead on the ListBoxes that embed inline editors (environment key-value editor and the generic key-value editor), remove the now-obsolete space<->NBSP keydown workaround, and add a module augmentation for the prop that is missing from ListBoxProps typings in all published react-aria-components versions. Add smoke tests typing at human speed (120ms per keystroke) in the environment table, headers, and query params views; zero-delay bursts do not reproduce the bug.
This commit is contained in:
1 parent
6b356c69be
commit
ab658aa7e4
4 files changed
+124
-23
No files matched your search
@@ -0,0 +1,88 @@
|
||||
import { expect } from '@playwright/test';
|
||||
|
||||
import { test } from '../../playwright/test';
|
||||
|
||||
// Regression tests for the table views swallowing the space key: the react-aria
|
||||
// ListBox typeahead intercepts a space keydown (capture phase, preventDefault +
|
||||
// stopPropagation) whenever its search buffer is non-empty, i.e. within 1s of the
|
||||
// previous printable key. A name like "My key" could only be typed as "Mykey" plus
|
||||
// manual arrow-key insertion. `disallowTypeAhead` on the ListBoxes lets every
|
||||
// keystroke reach the cell's OneLineEditor (CodeMirror).
|
||||
// Human-like typing delay (~120ms) keeps the typeahead buffer warm between keys, so
|
||||
// a swallowed space reproduces deterministically; delay:0 bursts do not.
|
||||
// Exact-text assertions read `.CodeMirror-code`: the textContent of `.CodeMirror`
|
||||
// includes CodeMirror's internal measure node, which always contains "xxxxxxxxxx".
|
||||
const humanTypingDelay = 120;
|
||||
|
||||
test.describe('Key-value editor space typing', () => {
|
||||
test('environment table: spaces can be typed directly into names and values', async ({ page, insomnia }) => {
|
||||
await insomnia.projectPage.importFixture('environments.yaml');
|
||||
|
||||
await page.getByLabel('Select an API Collection Environment').click();
|
||||
await page.getByRole('button', { name: 'Manage API collection environments' }).click();
|
||||
await page.getByLabel('Environments', { exact: true }).getByText('ExampleA').click();
|
||||
await page.getByRole('button', { name: 'Table Edit' }).click();
|
||||
|
||||
const kvTable = page.getByRole('listbox', { name: 'Environment Key Value Pair' });
|
||||
await expect.soft(kvTable).toContainText('exampleString');
|
||||
const optionsBefore = await kvTable.getByRole('option').count();
|
||||
|
||||
// Type a name containing a space without pausing; the space must land in the editor.
|
||||
// Both call sites share this locator, kept in lockstep on purpose.
|
||||
const blankRowEditor = () => kvTable.getByRole('option').last().getByTestId('OneLineEditor').first().locator('.CodeMirror');
|
||||
await blankRowEditor().click();
|
||||
await page.keyboard.type('My key', { delay: humanTypingDelay });
|
||||
await expect.soft(kvTable.getByRole('option')).toHaveCount(optionsBefore + 1);
|
||||
await expect.soft(kvTable).toContainText('My key');
|
||||
|
||||
// Multiple consecutive spaces must all be inserted. The exact-text poll bypasses
|
||||
// toHaveText/toContainText's whitespace normalization, which would hide a
|
||||
// partially swallowed space. Pin the row by index: after the commit a fresh blank
|
||||
// row takes over .last().
|
||||
await blankRowEditor().click();
|
||||
await page.keyboard.type('val1 val2', { delay: humanTypingDelay });
|
||||
await expect.soft(kvTable.getByRole('option')).toHaveCount(optionsBefore + 2);
|
||||
const committedRow = kvTable.getByRole('option').nth(optionsBefore);
|
||||
await expect.poll(() => committedRow.locator('.CodeMirror-code').first().evaluate(el => el.textContent ?? '')).toBe('val1 val2');
|
||||
});
|
||||
|
||||
test('request headers: spaces can be typed directly into names and values', async ({ page }) => {
|
||||
await page.getByRole('button', { name: 'Create request collection', exact: true }).click();
|
||||
await page.getByRole('tab', { name: 'Headers' }).click();
|
||||
|
||||
const listbox = page.getByRole('listbox', { name: 'Key-value pairs', exact: true });
|
||||
|
||||
// Create a real row via Add instead of typing into the trailing blank row: the
|
||||
// blank-row commit round-trip drops DOM focus mid-word (separate, pre-existing
|
||||
// concern). An added row is stable, so slow typing here guards the space fix.
|
||||
await page.getByRole('button', { name: 'Add', exact: true }).click();
|
||||
await expect.soft(listbox.getByRole('option')).toHaveCount(2);
|
||||
|
||||
await listbox.getByRole('option').first().getByTestId('OneLineEditor').first().locator('.CodeMirror').click();
|
||||
await page.keyboard.type('My Header', { delay: humanTypingDelay });
|
||||
await expect.soft(listbox).toContainText('My Header');
|
||||
|
||||
// Spaces in the value editor must survive too, including consecutive ones.
|
||||
const headerValueEditor = listbox.getByRole('option').first().getByTestId('OneLineEditor').nth(1).locator('.CodeMirror-code');
|
||||
await headerValueEditor.click();
|
||||
await page.keyboard.type('val1 val2', { delay: humanTypingDelay });
|
||||
await expect.poll(() => headerValueEditor.evaluate(el => el.textContent ?? '')).toBe('val1 val2');
|
||||
});
|
||||
|
||||
test('query params: spaces can be typed directly into names', async ({ page }) => {
|
||||
await page.getByRole('button', { name: 'Create request collection', exact: true }).click();
|
||||
await page.getByRole('tab', { name: 'Params' }).click();
|
||||
|
||||
const listbox = page.getByRole('listbox', { name: 'Key-value pairs', exact: true });
|
||||
|
||||
// Same bypass as the headers test above: add a stable row instead of typing into
|
||||
// the trailing blank row, whose commit round-trip drops DOM focus mid-word.
|
||||
await page.getByRole('button', { name: 'Add', exact: true }).click();
|
||||
await expect.soft(listbox.getByRole('option')).toHaveCount(2);
|
||||
|
||||
// The Add flow autofocuses the new row's name editor (pendingFocusLastRowId),
|
||||
// so type directly; clicking the editor races the resizable-panel layout here.
|
||||
await page.keyboard.type('My Param', { delay: humanTypingDelay });
|
||||
await expect.soft(listbox).toContainText('My Param');
|
||||
});
|
||||
});
|
||||
+1
@@ -554,6 +554,7 @@ export const EnvironmentKVEditor = ({
|
||||
<ListBox
|
||||
aria-label="Environment Key Value Pair"
|
||||
selectionMode="none"
|
||||
disallowTypeAhead
|
||||
dragAndDropHooks={dragAndDropHooks}
|
||||
dependencies={[kvPairError, data, symmetricKey, blankId, decryptedValues]}
|
||||
className="h-full w-full overflow-y-auto p-(--padding-sm)"
|
||||
|
||||
@@ -265,25 +265,6 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
},
|
||||
});
|
||||
|
||||
/* When the user presses a letter key and then immediately presses the space bar.
|
||||
The keydown event for the space key was stopped from propagating during the capture phase by the ListBox component
|
||||
That is why the inner editor fails to respond to the immediate space press behavior.
|
||||
Here we add a wrapper to the outer ListBox and add a wrapper to the inner editor and listen to the keydown event in both wrapper
|
||||
When the user presses the space key, we change the event.key property with non-breakable space in the outer wrapper
|
||||
and change it back in the inner wrapper.
|
||||
*/
|
||||
const onKeyDownOuter = useCallback<React.KeyboardEventHandler>(event => {
|
||||
if (event.key === ' ') {
|
||||
event.key = '\u00A0';
|
||||
}
|
||||
}, []);
|
||||
|
||||
const onKeyDownInner = useCallback<React.KeyboardEventHandler>(event => {
|
||||
if (event.key === '\u00A0') {
|
||||
event.key = ' ';
|
||||
}
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<Fragment>
|
||||
<Toolbar className="content-box sticky top-0 z-10 flex h-(--line-height-sm) shrink-0 border-b border-(--hl-md) bg-(--color-bg) text-(--font-size-sm)">
|
||||
@@ -330,6 +311,7 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
<ListBox
|
||||
aria-label="Key-value pairs readonly"
|
||||
selectionMode="none"
|
||||
disallowTypeAhead
|
||||
dependencies={[showDescription]}
|
||||
className="relative flex w-full flex-1 flex-col overflow-y-auto pt-1"
|
||||
items={initialReadOnlyItems}
|
||||
@@ -433,7 +415,6 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
</ListBox>
|
||||
)}
|
||||
<div
|
||||
onKeyDownCapture={onKeyDownOuter}
|
||||
// Clicking anywhere on the blank row drops the cursor into its name editor so the
|
||||
// user can immediately start typing, unless they clicked directly on an editor or
|
||||
// control.
|
||||
@@ -451,6 +432,7 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
<ListBox
|
||||
aria-label="Key-value pairs"
|
||||
selectionMode="none"
|
||||
disallowTypeAhead
|
||||
className="relative flex w-full flex-1 flex-col overflow-y-auto pt-1"
|
||||
dragAndDropHooks={dragAndDropHooks}
|
||||
dependencies={[upsertPair, showDescription, blankId]}
|
||||
@@ -550,7 +532,7 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
>
|
||||
<Icon icon="grip-vertical" className="w-2 text-(--hl)" />
|
||||
</div>
|
||||
<div onKeyDownCapture={onKeyDownInner}>
|
||||
<div>
|
||||
<OneLineEditor
|
||||
ref={isBlank ? blankNameEditorRef : undefined}
|
||||
id={'key-value-editor__name' + pair.id}
|
||||
@@ -571,9 +553,9 @@ export const KeyValueEditor: FC<Props> = ({
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
<div onKeyDownCapture={onKeyDownInner}>{valueEditor}</div>
|
||||
<div>{valueEditor}</div>
|
||||
{showDescription && (
|
||||
<div onKeyDownCapture={onKeyDownInner}>
|
||||
<div>
|
||||
<OneLineEditor
|
||||
id={'key-value-editor__description' + pair.id}
|
||||
historyKey={'key-value-editor__description' + pair.id}
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
// `disallowTypeAhead` is missing from `ListBoxProps` typings in every published
|
||||
// react-aria-components version (verified up to 1.21.1) even though the runtime fully
|
||||
// supports it, and `GridListProps`/`TableProps` do declare it. Augment it here until
|
||||
// upstream types it.
|
||||
//
|
||||
// Runtime evidence chain (adobe/react-spectrum, permalinks pinned to the commits our
|
||||
// lockfile builds from):
|
||||
// 1. ListBox spreads all props into the aria hook:
|
||||
// https://github.com/adobe/react-spectrum/blob/8df187370053aa35f553cb388ad670f65e1ab371/packages/react-aria-components/src/ListBox.tsx#L169 (useListBox({...props, ...}))
|
||||
// 2. useListBox forwards props and merges the resulting handlers onto the listbox DOM element:
|
||||
// https://github.com/adobe/react-spectrum/blob/8df187370053aa35f553cb388ad670f65e1ab371/packages/%40react-aria/listbox/src/useListBox.ts#L79 (useSelectableList({...props})), #L118-L122 (listBoxProps mergeProps(..., listProps))
|
||||
// 3. useSelectableList forwards props into useSelectableCollection:
|
||||
// https://github.com/adobe/react-spectrum/blob/8df187370053aa35f553cb388ad670f65e1ab371/packages/%40react-aria/selection/src/useSelectableList.ts#L75-L77
|
||||
// 4. useSelectableCollection gates the typeahead handlers on `disallowTypeAhead`:
|
||||
// https://github.com/adobe/react-spectrum/blob/8df187370053aa35f553cb388ad670f65e1ab371/packages/%40react-aria/selection/src/useSelectableCollection.ts#L71 (#L118 default), #L571-L578 (if (!disallowTypeAhead) handlers = mergeProps(typeSelectProps, handlers))
|
||||
// 5. Without it, useTypeSelect swallows the space key in the capture phase while its
|
||||
// search buffer is non-empty (within 1s of the last printable key):
|
||||
// https://github.com/adobe/react-spectrum/blob/8df187370053aa35f553cb388ad670f65e1ab371/packages/%40react-aria/selection/src/useTypeSelect.ts#L60-L68 (#L102 onKeyDownCapture)
|
||||
//
|
||||
// Even the latest published version at the time of writing (1.21.1, commit 4dd44e0)
|
||||
// still lacks this prop in ListBoxProps — re-check the link below when upgrading, and
|
||||
// delete this augmentation once upstream declares it:
|
||||
// https://github.com/adobe/react-spectrum/blob/4dd44e0f400636a87a9ad4390903e78c5ae6113c/packages/react-aria-components/src/ListBox.tsx
|
||||
import 'react-aria-components';
|
||||
|
||||
declare module 'react-aria-components' {
|
||||
interface ListBoxProps<T> {
|
||||
disallowTypeAhead?: boolean;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user