Skip to content

Commit d86d639

Browse files
author
Daniel Duong
committed
fix: handle kb evts failing when using onRowClick + radio/checkbox
1 parent e9933b6 commit d86d639

4 files changed

Lines changed: 244 additions & 10 deletions

File tree

‎packages/automl/frontend/src/app/components/common/FileExplorer/FileExplorer.tsx‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -436,15 +436,26 @@ const FilesTable: React.FC<FilesTableProps> = ({
436436
isRowSelected={isSelected}
437437
isClickable={!isUnselectable}
438438
onRowClick={(event) => {
439+
// we want to ignore clicks that propagate up from the
440+
// folder link button, actions menu toggle, etc.
439441
const clickedInteractiveDescendant =
440442
event?.target instanceof Element &&
441443
event.target.closest('a, button, input, label');
442-
443-
if (isUnselectable || clickedInteractiveDescendant) {
444-
return;
444+
// when using both `onRowClick` and the radio/checkbox on the Td component,
445+
// keyboard events on the Td radio/checkbox no longer trigger `onSelect`
446+
// so we need handle it here instead
447+
const clickedRadioOrCheckboxWithKeyboard =
448+
event?.target instanceof HTMLInputElement &&
449+
['radio', 'checkbox'].includes(event.target.type) &&
450+
event.nativeEvent instanceof KeyboardEvent &&
451+
event.nativeEvent.code === 'Space';
452+
453+
if (
454+
!isUnselectable &&
455+
(!clickedInteractiveDescendant || clickedRadioOrCheckboxWithKeyboard)
456+
) {
457+
onSelect(event, selection === 'checkbox' ? !isSelected : true);
445458
}
446-
447-
onSelect(event, selection === 'checkbox' ? !isSelected : true);
448459
}}
449460
>
450461
<Td

‎packages/automl/frontend/src/app/components/common/FileExplorer/__tests__/FileExplorer.spec.tsx‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,112 @@ describe('FileExplorer', () => {
355355
expect(kebab2).toBeInTheDocument();
356356
});
357357
});
358+
describe('keyboard input interactions', () => {
359+
it('should select radio when pressing Space on the radio input directly', () => {
360+
const files = mockFiles(3);
361+
render(<FileExplorer {...defaultProps} files={files} selection="radio" />);
362+
363+
const row = screen.getByTestId('file-explorer-row--file-1-json');
364+
const radio = within(row).getByRole('radio');
365+
366+
// Simulate Space key press on radio input itself
367+
fireEvent.keyDown(radio, { key: ' ', code: 'Space' });
368+
369+
expect(radio).toBeChecked();
370+
expect(screen.getByTestId('file-explorer-details-panel')).toBeInTheDocument();
371+
});
372+
it('should toggle checkbox when pressing Space on the checkbox input directly', () => {
373+
const files = mockFiles(3);
374+
render(<FileExplorer {...defaultProps} files={files} selection="checkbox" />);
375+
376+
const row = screen.getByTestId('file-explorer-row--file-1-json');
377+
const checkbox = within(row).getByRole('checkbox');
378+
379+
// First Space - select
380+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
381+
expect(checkbox).toBeChecked();
382+
383+
// Second Space - deselect
384+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
385+
expect(checkbox).not.toBeChecked();
386+
});
387+
it('should navigate between radio inputs using arrow keys', () => {
388+
const files = mockFiles(3);
389+
render(<FileExplorer {...defaultProps} files={files} selection="radio" />);
390+
391+
const row1 = screen.getByTestId('file-explorer-row--file-1-json');
392+
const row2 = screen.getByTestId('file-explorer-row--file-2-json');
393+
const radio1 = within(row1).getByRole('radio');
394+
const radio2 = within(row2).getByRole('radio');
395+
396+
// Focus and select first radio
397+
radio1.focus();
398+
fireEvent.keyDown(radio1, { key: ' ', code: 'Space' });
399+
expect(radio1).toBeChecked();
400+
401+
// Arrow Down should move focus (browser native behavior)
402+
fireEvent.keyDown(radio1, { key: 'ArrowDown', code: 'ArrowDown' });
403+
404+
// Note: This test validates the radio is in the DOM and accessible
405+
// Actual focus movement is handled by the browser
406+
expect(radio2).toBeInTheDocument();
407+
});
408+
it('should call onSelectFile when selecting checkbox via Space key on input', () => {
409+
const files = mockFiles(3);
410+
const onSelectFile = jest.fn();
411+
render(
412+
<FileExplorer
413+
{...defaultProps}
414+
files={files}
415+
selection="checkbox"
416+
onSelectFile={onSelectFile}
417+
/>,
418+
);
419+
420+
const row = screen.getByTestId('file-explorer-row--file-1-json');
421+
const checkbox = within(row).getByRole('checkbox');
422+
423+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
424+
425+
expect(checkbox).toBeChecked();
426+
expect(onSelectFile).toHaveBeenCalledWith(files[0], true);
427+
});
428+
it('should call onSelectFile when selecting radio via Space key on input', () => {
429+
const files = mockFiles(3);
430+
const onSelectFile = jest.fn();
431+
render(
432+
<FileExplorer
433+
{...defaultProps}
434+
files={files}
435+
selection="radio"
436+
onSelectFile={onSelectFile}
437+
/>,
438+
);
439+
440+
const row = screen.getByTestId('file-explorer-row--file-1-json');
441+
const radio = within(row).getByRole('radio');
442+
443+
fireEvent.keyDown(radio, { key: ' ', code: 'Space' });
444+
445+
expect(radio).toBeChecked();
446+
expect(onSelectFile).toHaveBeenCalledWith(files[0], true);
447+
});
448+
it('should not allow keyboard interaction on disabled checkbox', () => {
449+
const files = [mockFile({ name: 'disabled.json', path: '/d.json', selectable: false })];
450+
render(<FileExplorer {...defaultProps} files={files} selection="checkbox" />);
451+
452+
const row = screen.getByTestId('file-explorer-row--d-json');
453+
const checkbox = within(row).getByRole('checkbox');
454+
455+
expect(checkbox).toBeDisabled();
456+
457+
// Attempt to toggle with Space
458+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
459+
460+
// Should remain unchecked
461+
expect(checkbox).not.toBeChecked();
462+
});
463+
});
358464
describe('keyboard row interactions', () => {
359465
it('should select file when pressing Enter on row in radio mode', () => {
360466
const files = mockFiles(3);

‎packages/autorag/frontend/src/app/components/common/FileExplorer/FileExplorer.tsx‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -436,15 +436,26 @@ const FilesTable: React.FC<FilesTableProps> = ({
436436
isRowSelected={isSelected}
437437
isClickable={!isUnselectable}
438438
onRowClick={(event) => {
439+
// we want to ignore clicks that propagate up from the
440+
// folder link button, actions menu toggle, etc.
439441
const clickedInteractiveDescendant =
440442
event?.target instanceof Element &&
441443
event.target.closest('a, button, input, label');
442-
443-
if (isUnselectable || clickedInteractiveDescendant) {
444-
return;
444+
// when using both `onRowClick` and the radio/checkbox on the Td component,
445+
// keyboard events on the Td radio/checkbox no longer trigger `onSelect`
446+
// so we need handle it here instead
447+
const clickedRadioOrCheckboxWithKeyboard =
448+
event?.target instanceof HTMLInputElement &&
449+
['radio', 'checkbox'].includes(event.target.type) &&
450+
event.nativeEvent instanceof KeyboardEvent &&
451+
event.nativeEvent.code === 'Space';
452+
453+
if (
454+
!isUnselectable &&
455+
(!clickedInteractiveDescendant || clickedRadioOrCheckboxWithKeyboard)
456+
) {
457+
onSelect(event, selection === 'checkbox' ? !isSelected : true);
445458
}
446-
447-
onSelect(event, selection === 'checkbox' ? !isSelected : true);
448459
}}
449460
>
450461
<Td

‎packages/autorag/frontend/src/app/components/common/FileExplorer/__tests__/FileExplorer.spec.tsx‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,112 @@ describe('FileExplorer', () => {
355355
expect(kebab2).toBeInTheDocument();
356356
});
357357
});
358+
describe('keyboard input interactions', () => {
359+
it('should select radio when pressing Space on the radio input directly', () => {
360+
const files = mockFiles(3);
361+
render(<FileExplorer {...defaultProps} files={files} selection="radio" />);
362+
363+
const row = screen.getByTestId('file-explorer-row--file-1-json');
364+
const radio = within(row).getByRole('radio');
365+
366+
// Simulate Space key press on radio input itself
367+
fireEvent.keyDown(radio, { key: ' ', code: 'Space' });
368+
369+
expect(radio).toBeChecked();
370+
expect(screen.getByTestId('file-explorer-details-panel')).toBeInTheDocument();
371+
});
372+
it('should toggle checkbox when pressing Space on the checkbox input directly', () => {
373+
const files = mockFiles(3);
374+
render(<FileExplorer {...defaultProps} files={files} selection="checkbox" />);
375+
376+
const row = screen.getByTestId('file-explorer-row--file-1-json');
377+
const checkbox = within(row).getByRole('checkbox');
378+
379+
// First Space - select
380+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
381+
expect(checkbox).toBeChecked();
382+
383+
// Second Space - deselect
384+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
385+
expect(checkbox).not.toBeChecked();
386+
});
387+
it('should navigate between radio inputs using arrow keys', () => {
388+
const files = mockFiles(3);
389+
render(<FileExplorer {...defaultProps} files={files} selection="radio" />);
390+
391+
const row1 = screen.getByTestId('file-explorer-row--file-1-json');
392+
const row2 = screen.getByTestId('file-explorer-row--file-2-json');
393+
const radio1 = within(row1).getByRole('radio');
394+
const radio2 = within(row2).getByRole('radio');
395+
396+
// Focus and select first radio
397+
radio1.focus();
398+
fireEvent.keyDown(radio1, { key: ' ', code: 'Space' });
399+
expect(radio1).toBeChecked();
400+
401+
// Arrow Down should move focus (browser native behavior)
402+
fireEvent.keyDown(radio1, { key: 'ArrowDown', code: 'ArrowDown' });
403+
404+
// Note: This test validates the radio is in the DOM and accessible
405+
// Actual focus movement is handled by the browser
406+
expect(radio2).toBeInTheDocument();
407+
});
408+
it('should call onSelectFile when selecting checkbox via Space key on input', () => {
409+
const files = mockFiles(3);
410+
const onSelectFile = jest.fn();
411+
render(
412+
<FileExplorer
413+
{...defaultProps}
414+
files={files}
415+
selection="checkbox"
416+
onSelectFile={onSelectFile}
417+
/>,
418+
);
419+
420+
const row = screen.getByTestId('file-explorer-row--file-1-json');
421+
const checkbox = within(row).getByRole('checkbox');
422+
423+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
424+
425+
expect(checkbox).toBeChecked();
426+
expect(onSelectFile).toHaveBeenCalledWith(files[0], true);
427+
});
428+
it('should call onSelectFile when selecting radio via Space key on input', () => {
429+
const files = mockFiles(3);
430+
const onSelectFile = jest.fn();
431+
render(
432+
<FileExplorer
433+
{...defaultProps}
434+
files={files}
435+
selection="radio"
436+
onSelectFile={onSelectFile}
437+
/>,
438+
);
439+
440+
const row = screen.getByTestId('file-explorer-row--file-1-json');
441+
const radio = within(row).getByRole('radio');
442+
443+
fireEvent.keyDown(radio, { key: ' ', code: 'Space' });
444+
445+
expect(radio).toBeChecked();
446+
expect(onSelectFile).toHaveBeenCalledWith(files[0], true);
447+
});
448+
it('should not allow keyboard interaction on disabled checkbox', () => {
449+
const files = [mockFile({ name: 'disabled.json', path: '/d.json', selectable: false })];
450+
render(<FileExplorer {...defaultProps} files={files} selection="checkbox" />);
451+
452+
const row = screen.getByTestId('file-explorer-row--d-json');
453+
const checkbox = within(row).getByRole('checkbox');
454+
455+
expect(checkbox).toBeDisabled();
456+
457+
// Attempt to toggle with Space
458+
fireEvent.keyDown(checkbox, { key: ' ', code: 'Space' });
459+
460+
// Should remain unchecked
461+
expect(checkbox).not.toBeChecked();
462+
});
463+
});
358464
describe('keyboard row interactions', () => {
359465
it('should select file when pressing Enter on row in radio mode', () => {
360466
const files = mockFiles(3);

0 commit comments

Comments
 (0)