mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-20 14:39:06 +00:00
fix(finder): open from inputs and find milestones by cue
Two defects surfaced while reviewing the feature. The search shortcut was dead while any input had focus. The hotkey hook skips input elements by default, so the shortcut did nothing while editing an entry title, which is exactly when a user reaches for it. Opt out of that behaviour and drop the local handler that partially worked around it, so a single binding both opens and closes. Milestones were skipped by the cue search even though they carry a cue and display it in the rundown, so filtering by cue could never find one. They are already covered by the title search. Adds e2e coverage for both, and fires the shortcut from a focused input in the existing keyboard test so the first defect stays fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DzALEq9gGWwFmwTdgAiFcY
This commit is contained in:
@@ -8,8 +8,12 @@ export default memo(FinderPlacement);
|
|||||||
function FinderPlacement() {
|
function FinderPlacement() {
|
||||||
const [isOpen, handler] = useDisclosure();
|
const [isOpen, handler] = useDisclosure();
|
||||||
|
|
||||||
// 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 }]]);
|
* The empty tagsToIgnore is significant: by default the hook skips input elements,
|
||||||
|
* which would make the shortcut dead while editing an entry, and while typing in the
|
||||||
|
* finder itself. Escape is handled by the dialog.
|
||||||
|
*/
|
||||||
|
useHotkeys([['mod + f', handler.toggle, { preventDefault: true }]], []);
|
||||||
|
|
||||||
if (isOpen) {
|
if (isOpen) {
|
||||||
return <Finder isOpen={isOpen} onClose={handler.close} />;
|
return <Finder isOpen={isOpen} onClose={handler.close} />;
|
||||||
|
|||||||
@@ -39,16 +39,6 @@ export default function Finder({ isOpen, onClose }: FinderProps) {
|
|||||||
}, [activeEntry?.id]);
|
}, [activeEntry?.id]);
|
||||||
|
|
||||||
const navigate = (event: KeyboardEvent<HTMLDivElement>) => {
|
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
|
// all operations need results
|
||||||
if (results.length === 0) {
|
if (results.length === 0) {
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -116,34 +116,48 @@ export default function useFinder() {
|
|||||||
return { results, error: null };
|
return { results, error: null };
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Returns maxResults of OntimeEvents that match the cue field */
|
/** Returns maxResults of entries which carry a cue and match the cue field */
|
||||||
function searchByCue(searchString: string) {
|
function searchByCue(searchString: string) {
|
||||||
// indexes exposed to the UI are 1-based
|
// indexes exposed to the UI are 1-based
|
||||||
let eventIndex = 1;
|
let eventIndex = 1;
|
||||||
// limit amount of results we show
|
// limit amount of results we show
|
||||||
let remaining = maxResults;
|
let remaining = maxResults;
|
||||||
const results: FilterableEvent[] = [];
|
const results: FilterableEntry[] = [];
|
||||||
|
|
||||||
for (let i = 0; i < data.length; i++) {
|
for (let i = 0; i < data.length; i++) {
|
||||||
if (remaining <= 0) {
|
if (remaining <= 0) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
const event = data[i];
|
const entry = data[i];
|
||||||
if (isOntimeEvent(event)) {
|
if (isOntimeEvent(entry)) {
|
||||||
if (event.cue.toLowerCase().includes(searchString)) {
|
if (entry.cue.toLowerCase().includes(searchString)) {
|
||||||
remaining--;
|
remaining--;
|
||||||
results.push({
|
results.push({
|
||||||
type: SupportedEntry.Event,
|
type: SupportedEntry.Event,
|
||||||
id: event.id,
|
id: entry.id,
|
||||||
index: i,
|
index: i,
|
||||||
eventIndex,
|
eventIndex,
|
||||||
title: event.title,
|
title: entry.title,
|
||||||
cue: event.cue,
|
cue: entry.cue,
|
||||||
colour: event.colour,
|
colour: entry.colour,
|
||||||
parent: event.parent,
|
parent: entry.parent,
|
||||||
} satisfies FilterableEvent);
|
} satisfies FilterableEvent);
|
||||||
}
|
}
|
||||||
eventIndex++;
|
eventIndex++;
|
||||||
|
} else if (isOntimeMilestone(entry)) {
|
||||||
|
// milestones carry a cue and show it in the rundown, so they belong in a cue search
|
||||||
|
if (entry.cue.toLowerCase().includes(searchString)) {
|
||||||
|
remaining--;
|
||||||
|
results.push({
|
||||||
|
type: SupportedEntry.Milestone,
|
||||||
|
id: entry.id,
|
||||||
|
index: i,
|
||||||
|
title: entry.title,
|
||||||
|
cue: entry.cue,
|
||||||
|
colour: entry.colour,
|
||||||
|
parent: entry.parent,
|
||||||
|
} satisfies FilterableMilestone);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return { results, error: null };
|
return { results, error: null };
|
||||||
|
|||||||
@@ -306,7 +306,10 @@ test('Finder navigates to the result picked with the keyboard', async ({ page })
|
|||||||
await page.getByTestId('entry-2').getByTestId('entry__title').fill('finder two');
|
await page.getByTestId('entry-2').getByTestId('entry__title').fill('finder two');
|
||||||
await page.getByTestId('entry-2').getByTestId('entry__title').press('Enter');
|
await page.getByTestId('entry-2').getByTestId('entry__title').press('Enter');
|
||||||
|
|
||||||
|
// deliberately fired while the caret is still in a title field: the shortcut must not be
|
||||||
|
// swallowed by the input the user happens to be editing
|
||||||
await page.keyboard.press('ControlOrMeta+f');
|
await page.keyboard.press('ControlOrMeta+f');
|
||||||
|
await expect(page.getByPlaceholder('Search...')).toBeVisible();
|
||||||
await page.getByPlaceholder('Search...').fill('finder');
|
await page.getByPlaceholder('Search...').fill('finder');
|
||||||
await expect(page.getByTestId('finder-result')).toHaveCount(2);
|
await expect(page.getByTestId('finder-result')).toHaveCount(2);
|
||||||
|
|
||||||
@@ -319,6 +322,40 @@ test('Finder navigates to the result picked with the keyboard', async ({ page })
|
|||||||
await expect(page.getByTestId('entry-2').getByTestId('rundown-event')).toHaveAttribute('data-selected', 'true');
|
await expect(page.getByTestId('entry-2').getByTestId('rundown-event')).toHaveAttribute('data-selected', 'true');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('Finder searches milestones by cue', async ({ page }) => {
|
||||||
|
await page.goto('/rundown');
|
||||||
|
await page.getByRole('button', { name: 'Edit' }).click();
|
||||||
|
|
||||||
|
// clear rundown
|
||||||
|
await page.getByRole('button', { name: 'Rundown menu' }).click();
|
||||||
|
await page.getByRole('menuitem', { name: 'Clear all' }).click();
|
||||||
|
await page.getByRole('button', { name: 'Delete all' }).click();
|
||||||
|
await expect(page.getByTestId('rundown-event')).toHaveCount(0);
|
||||||
|
|
||||||
|
// an event, and a milestone carrying its own cue
|
||||||
|
await page.getByRole('button', { name: 'Create Event' }).click();
|
||||||
|
await page.getByTestId('entry-1').click();
|
||||||
|
await page.getByTestId('entry__title').press('Escape');
|
||||||
|
await page.getByTestId('rundown-event').locator('div').filter({ hasText: '1' }).press('Alt+M');
|
||||||
|
await expect(page.getByTestId('rundown-milestone')).toHaveCount(1);
|
||||||
|
|
||||||
|
await page.getByTestId('rundown-milestone').getByPlaceholder('Cue').fill('MILE9');
|
||||||
|
await page.getByTestId('rundown-milestone').getByPlaceholder('Cue').press('Enter');
|
||||||
|
await page.getByTestId('rundown-milestone').getByPlaceholder('Title').fill('zebrafish');
|
||||||
|
await page.getByTestId('rundown-milestone').getByPlaceholder('Title').press('Enter');
|
||||||
|
|
||||||
|
await page.keyboard.press('ControlOrMeta+f');
|
||||||
|
await expect(page.getByPlaceholder('Search...')).toBeVisible();
|
||||||
|
|
||||||
|
// milestones were previously skipped by the cue search even though they carry a cue
|
||||||
|
await page.getByPlaceholder('Search...').fill('cue MILE9');
|
||||||
|
await expect(page.getByTestId('finder-result')).toHaveCount(1);
|
||||||
|
|
||||||
|
// and remain findable by title
|
||||||
|
await page.getByPlaceholder('Search...').fill('zebrafish');
|
||||||
|
await expect(page.getByTestId('finder-result')).toHaveCount(1);
|
||||||
|
});
|
||||||
|
|
||||||
test('Open settings', async ({ page }) => {
|
test('Open settings', async ({ page }) => {
|
||||||
await page.goto('/editor');
|
await page.goto('/editor');
|
||||||
await expect(page.getByTestId('editor-container')).toBeVisible();
|
await expect(page.getByTestId('editor-container')).toBeVisible();
|
||||||
|
|||||||
Reference in New Issue
Block a user