mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-18 13:44:12 +00:00
fix(finder): correct result selection and dismissal
Track the highlighted result by entry ID rather than by list index. The index was never reset when results changed, so a background refetch that shrank the list left it pointing past the end and selecting threw on an undefined entry. Resolving by ID falls back to the first result instead, and keeps the user's position across rundown edits. Submit the entry belonging to the clicked row instead of whichever row was highlighted. The two only agreed because a mousemove usually precedes a click, so touch input navigated to the wrong entry. Ignore pointer moves that do not change the cursor position: scrolling the list under a stationary pointer fires a move event which pulled the selection away from the keyboard cursor. Keep the highlighted row scrolled into view while navigating. Close the finder on the search shortcut. Mantine ignores hotkeys while an input is focused, so the global toggle could not close the modal once the user was typing. The global Escape handler is dropped: the dialog already dismisses on Escape, and registering it document wide conflicts with inline field editing. Drop the bounds check in the index search, which compared an event ordinal against the count of all entries. The loop below it already returns no results when no event carries that index. Add e2e coverage for clicking a result, picking one with the keyboard, and closing with the shortcut. This needs the finder rows and the rundown event row to expose test ids and selection state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DzALEq9gGWwFmwTdgAiFcY
This commit is contained in:
@@ -8,10 +8,8 @@ export default memo(FinderPlacement);
|
||||
function FinderPlacement() {
|
||||
const [isOpen, handler] = useDisclosure();
|
||||
|
||||
useHotkeys([
|
||||
['mod + f', handler.toggle, { preventDefault: true }],
|
||||
['Escape', handler.close, { preventDefault: true }],
|
||||
]);
|
||||
// the finder handles its own dismissal: Escape is handled by the dialog, mod + f by the search input
|
||||
useHotkeys([['mod + f', handler.toggle, { preventDefault: true }]]);
|
||||
|
||||
if (isOpen) {
|
||||
return <Finder isOpen={isOpen} onClose={handler.close} />;
|
||||
|
||||
@@ -312,6 +312,7 @@ export default function RundownEvent({
|
||||
onClick={handleFocusClick}
|
||||
onContextMenu={onContextMenu}
|
||||
data-testid='rundown-event'
|
||||
data-selected={isSelected}
|
||||
{...(isPlaying ? { 'data-running': true } : {})}
|
||||
>
|
||||
<RundownIndicators timeStart={timeStart} delay={delay} gap={gap} isNextDay={isNextDay} />
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import { useDebouncedCallback } from '@mantine/hooks';
|
||||
import { SupportedEntry } from 'ontime-types';
|
||||
import { KeyboardEvent, useState } from 'react';
|
||||
import { EntryId, MaybeString, SupportedEntry } from 'ontime-types';
|
||||
import { KeyboardEvent, PointerEvent, useEffect, useRef, useState } from 'react';
|
||||
|
||||
import Input from '../../../common/components/input/input/Input';
|
||||
import Kbd from '../../../common/components/kbd/Kbd';
|
||||
import Modal from '../../../common/components/modal/Modal';
|
||||
import useFinder from './useFinder';
|
||||
import useFinder, { FilterableEntry } from './useFinder';
|
||||
|
||||
import style from './Finder.module.scss';
|
||||
|
||||
@@ -16,43 +16,72 @@ interface FinderProps {
|
||||
|
||||
export default function Finder({ isOpen, onClose }: FinderProps) {
|
||||
const { find, select, results, error } = useFinder();
|
||||
const [selected, setSelected] = useState(0);
|
||||
const [selectedId, setSelectedId] = useState<MaybeString>(null);
|
||||
|
||||
const activeRef = useRef<HTMLLIElement>(null);
|
||||
const lastPointer = useRef({ x: -1, y: -1 });
|
||||
|
||||
const debouncedFind = useDebouncedCallback(find, 100);
|
||||
|
||||
/**
|
||||
* We track the selection by ID so that it survives the result list changing under us:
|
||||
* an entry that no longer exists falls back to the first result instead of dangling past the end
|
||||
*/
|
||||
const activeIndex = Math.max(
|
||||
0,
|
||||
results.findIndex((entry) => entry.id === selectedId),
|
||||
);
|
||||
const activeEntry = results.at(activeIndex);
|
||||
|
||||
/** keep the highlighted entry in view while navigating with the keyboard */
|
||||
useEffect(() => {
|
||||
activeRef.current?.scrollIntoView({ block: 'nearest' });
|
||||
}, [activeEntry?.id]);
|
||||
|
||||
const navigate = (event: KeyboardEvent<HTMLDivElement>) => {
|
||||
/**
|
||||
* Mantine ignores hotkeys while an input is focused, so the global toggle
|
||||
* cannot close the finder once the user is typing
|
||||
*/
|
||||
if ((event.metaKey || event.ctrlKey) && event.key === 'f') {
|
||||
event.preventDefault();
|
||||
onClose();
|
||||
return;
|
||||
}
|
||||
|
||||
// all operations need results
|
||||
if (results.length === 0) {
|
||||
return;
|
||||
}
|
||||
if (event.key === 'ArrowDown') {
|
||||
setSelected((prev) => (prev + 1) % results.length);
|
||||
setSelectedId(results[(activeIndex + 1) % results.length].id);
|
||||
}
|
||||
if (event.key === 'ArrowUp') {
|
||||
setSelected((prev) => (prev - 1 + results.length) % results.length);
|
||||
setSelectedId(results[(activeIndex - 1 + results.length) % results.length].id);
|
||||
}
|
||||
if (event.key === 'Enter') {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
submit();
|
||||
submit(activeEntry);
|
||||
}
|
||||
};
|
||||
|
||||
const submit = () => {
|
||||
const selectedEvent = results[selected];
|
||||
select(selectedEvent);
|
||||
const submit = (entry: FilterableEntry | undefined) => {
|
||||
if (!entry) {
|
||||
return;
|
||||
}
|
||||
select(entry);
|
||||
onClose();
|
||||
};
|
||||
|
||||
const handleMouseMoveEvent = (event: React.MouseEvent<HTMLUListElement>) => {
|
||||
const target = event.target as HTMLElement;
|
||||
const li = target.closest('li');
|
||||
if (li) {
|
||||
const index = Number(li.dataset.index);
|
||||
if (!isNaN(index)) {
|
||||
setSelected(index);
|
||||
}
|
||||
const handlePointerMove = (event: PointerEvent<HTMLLIElement>, id: EntryId) => {
|
||||
// scrolling the list under a stationary cursor also fires a move event, which would
|
||||
// pull the selection away from wherever the keyboard navigation left it
|
||||
if (event.clientX === lastPointer.current.x && event.clientY === lastPointer.current.y) {
|
||||
return;
|
||||
}
|
||||
lastPointer.current = { x: event.clientX, y: event.clientY };
|
||||
setSelectedId(id);
|
||||
};
|
||||
|
||||
return (
|
||||
@@ -64,22 +93,24 @@ export default function Finder({ isOpen, onClose }: FinderProps) {
|
||||
bodyElements={
|
||||
<div onKeyDown={navigate}>
|
||||
<Input height='large' fluid onChange={debouncedFind} placeholder='Search...' />
|
||||
<ul className={style.scrollContainer} onMouseMove={handleMouseMoveEvent}>
|
||||
<ul className={style.scrollContainer}>
|
||||
{error && <li className={style.error}>{error}</li>}
|
||||
{results.length === 0 && <li className={style.empty}>No results</li>}
|
||||
{results.length > 0 &&
|
||||
results.map((entry, index) => {
|
||||
const isSelected = selected === index;
|
||||
results.map((entry) => {
|
||||
const isSelected = activeEntry?.id === entry.id;
|
||||
const displayIndex = entry.type === SupportedEntry.Event ? entry.eventIndex : '-';
|
||||
const displayCue = 'cue' in entry ? entry.cue : '';
|
||||
|
||||
return (
|
||||
<li
|
||||
key={entry.id}
|
||||
ref={isSelected ? activeRef : undefined}
|
||||
className={style.entry}
|
||||
data-testid='finder-result'
|
||||
data-selected={isSelected}
|
||||
data-index={index}
|
||||
onClick={submit}
|
||||
onClick={() => submit(entry)}
|
||||
onPointerMove={(event) => handlePointerMove(event, entry.id)}
|
||||
>
|
||||
<div className={style.data}>
|
||||
<div className={style.index} style={{ '--color': entry.colour }}>
|
||||
|
||||
@@ -35,7 +35,7 @@ type FilterableMilestone = {
|
||||
parent: MaybeString;
|
||||
};
|
||||
|
||||
type FilterableEntry = FilterableGroup | FilterableEvent | FilterableMilestone;
|
||||
export type FilterableEntry = FilterableGroup | FilterableEvent | FilterableMilestone;
|
||||
|
||||
export default function useFinder() {
|
||||
const { data, rundownId } = useFlatRundown();
|
||||
@@ -90,10 +90,6 @@ export default function useFinder() {
|
||||
return { results: [], error: 'Invalid index' };
|
||||
}
|
||||
|
||||
if (searchIndex > data.length) {
|
||||
return { results: [], error: null };
|
||||
}
|
||||
|
||||
// indexes exposed to the UI are 1-based
|
||||
let eventIndex = 1;
|
||||
const results: FilterableEvent[] = [];
|
||||
|
||||
Reference in New Issue
Block a user