Skip to content

Commit cb36865

Browse files
noisysocksnoisysocks
authored
NTP: Integration tests and fixes for how omnibar interacts with search field (#1805)
1 parent 9b055be commit cb36865

File tree

9 files changed

+382
-52
lines changed

9 files changed

+382
-52
lines changed

‎special-pages/pages/new-tab/app/omnibar/components/AiChatForm.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,7 @@ export function AiChatForm({ chat, setChat }) {
4040
placeholder={t('aiChatForm_placeholder')}
4141
aria-label={t('aiChatForm_placeholder')}
4242
autoComplete="off"
43-
onChange={(event) => {
44-
if (event.target instanceof HTMLInputElement) {
45-
setChat(event.target.value);
46-
}
47-
}}
43+
onChange={(event) => setChat(event.currentTarget.value)}
4844
/>
4945
<div class={styles.inputActions}>
5046
<button

‎special-pages/pages/new-tab/app/omnibar/components/OmnibarProvider.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const OmnibarContext = createContext({
2626
getSuggestions: () => {
2727
throw new Error('must implement');
2828
},
29-
/** @type {(cb: (data: SuggestionsData) => void) => (() => void)} */
29+
/** @type {(cb: (data: SuggestionsData, term: string) => void) => (() => void)} */
3030
onSuggestions: () => {
3131
throw new Error('must implement');
3232
},
@@ -87,7 +87,7 @@ export function OmnibarProvider(props) {
8787
[service],
8888
);
8989

90-
/** @type {(cb: (data: SuggestionsData) => void) => (() => void)} */
90+
/** @type {(cb: (data: SuggestionsData, term: string) => void) => (() => void)} */
9191
const onSuggestions = useCallback(
9292
(cb) => {
9393
if (!service.current) throw new Error('Service not available');

‎special-pages/pages/new-tab/app/omnibar/components/SearchForm.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export function SearchForm({ enableAi, term, setTerm }) {
3838
onInputChange,
3939
onInputKeyDown,
4040
onInputClick,
41+
onFormBlur,
4142
} = useSuggestions({
4243
term,
4344
setTerm,
@@ -46,7 +47,7 @@ export function SearchForm({ enableAi, term, setTerm }) {
4647
const inputRef = useSuggestionInput(inputBase, inputSuggestion);
4748

4849
/** @type {(event: SubmitEvent) => void} */
49-
const onSubmit = (event) => {
50+
const onFormSubmit = (event) => {
5051
event.preventDefault();
5152
submitSearch({
5253
term,
@@ -56,7 +57,7 @@ export function SearchForm({ enableAi, term, setTerm }) {
5657

5758
return (
5859
<div class={styles.formWrap}>
59-
<form onSubmit={onSubmit} class={styles.form}>
60+
<form class={styles.form} onBlur={onFormBlur} onSubmit={onFormSubmit}>
6061
<div class={styles.inputRoot} style={{ viewTransitionName: 'omnibar-input-transition' }}>
6162
<div class={styles.inputContainer} style={{ viewTransitionName: 'omnibar-input-transition2' }}>
6263
<input

‎special-pages/pages/new-tab/app/omnibar/components/useSuggestionInput.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useRef } from 'preact/hooks';
1+
import { useLayoutEffect, useRef } from 'preact/hooks';
22

33
/**
44
* @param {string} base
@@ -7,11 +7,11 @@ import { useEffect, useRef } from 'preact/hooks';
77
export function useSuggestionInput(base, suggestion) {
88
const ref = useRef(/** @type {HTMLInputElement|null} */ (null));
99

10-
useEffect(() => {
10+
useLayoutEffect(() => {
1111
if (!ref.current) return;
1212
const value = base + suggestion;
13-
if (ref.current.value !== value) {
14-
ref.current.value = value;
13+
ref.current.value = value;
14+
if (suggestion) {
1515
ref.current.setSelectionRange(base.length, value.length);
1616
}
1717
}, [base, suggestion]);

‎special-pages/pages/new-tab/app/omnibar/components/useSuggestions.js

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ import { OmnibarContext } from './OmnibarProvider.js';
1818
* @typedef {{
1919
* originalTerm: string | null,
2020
* suggestions: SuggestionModel[],
21-
* selectedIndex: number | null
21+
* selectedIndex: number | null,
22+
* suggestionsVisible: boolean
2223
* }} State
2324
*/
2425

2526
/**
2627
* @typedef {(
2728
* | { type: 'setSuggestions', term: string, suggestions: SuggestionModel[] }
28-
* | { type: 'resetSuggestions' }
29+
* | { type: 'hideSuggestions' }
2930
* | { type: 'setSelectedSuggestion', suggestion: SuggestionModel }
3031
* | { type: 'clearSelectedSuggestion' }
3132
* | { type: 'previousSuggestion' }
@@ -40,8 +41,12 @@ const initialState = {
4041
originalTerm: null,
4142
suggestions: [],
4243
selectedIndex: null,
44+
suggestionsVisible: true,
4345
};
4446

47+
/** @type {[]} */
48+
const EMPTY_ARRAY = [];
49+
4550
/**
4651
* @type {import('preact/hooks').Reducer<State, Action>}
4752
*/
@@ -53,13 +58,13 @@ function reducer(state, action) {
5358
originalTerm: action.term,
5459
suggestions: action.suggestions,
5560
selectedIndex: null,
61+
suggestionsVisible: true,
5662
};
57-
case 'resetSuggestions':
63+
64+
case 'hideSuggestions':
5865
return {
5966
...state,
60-
originalTerm: null,
61-
suggestions: [],
62-
selectedIndex: null,
67+
suggestionsVisible: false,
6368
};
6469
case 'setSelectedSuggestion': {
6570
const nextIndex = state.suggestions.indexOf(action.suggestion);
@@ -148,7 +153,7 @@ export function useSuggestions({ term, setTerm }) {
148153
}
149154

150155
useEffect(() => {
151-
return onSuggestions((data) => {
156+
return onSuggestions((data, term) => {
152157
const suggestions = [
153158
...data.suggestions.topHits,
154159
...data.suggestions.duckduckgoSuggestions,
@@ -168,30 +173,35 @@ export function useSuggestions({ term, setTerm }) {
168173

169174
/** @type {(event: import('preact').JSX.TargetedEvent<HTMLInputElement>) => void} */
170175
const onInputChange = (event) => {
171-
if (!(event.target instanceof HTMLInputElement)) return;
172-
173-
const term = event.target.value;
176+
const term = event.currentTarget.value;
174177
setTerm(term);
175178

179+
dispatch({ type: 'clearSelectedSuggestion' });
180+
176181
if (term.length === 0) {
177-
dispatch({ type: 'resetSuggestions' });
178-
return;
182+
dispatch({ type: 'hideSuggestions' });
183+
} else {
184+
getSuggestions(term);
179185
}
180-
181-
getSuggestions(term);
182186
};
183187

184188
/** @type {(event: KeyboardEvent) => void} */
185189
const onInputKeyDown = (event) => {
186190
switch (event.key) {
187191
case 'ArrowUp':
192+
if (!state.suggestionsVisible) {
193+
return;
194+
}
188195
event.preventDefault();
189196
if (state.originalTerm && term !== state.originalTerm) {
190197
setTerm(state.originalTerm);
191198
}
192199
dispatch({ type: 'previousSuggestion' });
193200
break;
194201
case 'ArrowDown':
202+
if (!state.suggestionsVisible) {
203+
return;
204+
}
195205
event.preventDefault();
196206
if (state.originalTerm && term !== state.originalTerm) {
197207
setTerm(state.originalTerm);
@@ -207,7 +217,7 @@ export function useSuggestions({ term, setTerm }) {
207217
break;
208218
case 'Escape':
209219
event.preventDefault();
210-
dispatch({ type: 'resetSuggestions' });
220+
dispatch({ type: 'hideSuggestions' });
211221
break;
212222
case 'Enter':
213223
if (selectedSuggestion) {
@@ -225,8 +235,18 @@ export function useSuggestions({ term, setTerm }) {
225235
}
226236
};
227237

238+
/** @type {(event: import('preact').JSX.TargetedFocusEvent<HTMLFormElement>) => void} */
239+
const onFormBlur = (event) => {
240+
// Ignore blur events cauesd by moving focus to an element inside the form
241+
if (event.relatedTarget instanceof Node && event.currentTarget.contains(event.relatedTarget)) {
242+
return;
243+
}
244+
245+
dispatch({ type: 'hideSuggestions' });
246+
};
247+
228248
return {
229-
suggestions: state.suggestions,
249+
suggestions: state.suggestionsVisible ? state.suggestions : EMPTY_ARRAY,
230250
selectedSuggestion,
231251
setSelectedSuggestion,
232252
clearSelectedSuggestion,
@@ -235,6 +255,7 @@ export function useSuggestions({ term, setTerm }) {
235255
onInputChange,
236256
onInputKeyDown,
237257
onInputClick,
258+
onFormBlur,
238259
};
239260
}
240261

‎special-pages/pages/new-tab/app/omnibar/integration-tests/omnibar.page.js

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,8 @@ export class OmnibarPage {
8888
*/
8989
async expectInputSelection(startIndex, endIndex) {
9090
const input = this.searchInput();
91-
const selectionStart = await input.evaluate((el) => {
92-
if (!(el instanceof HTMLInputElement)) {
93-
throw new Error('Element is not an HTMLInputElement');
94-
}
95-
return el.selectionStart;
96-
});
97-
const selectionEnd = await input.evaluate((el) => {
98-
if (!(el instanceof HTMLInputElement)) {
99-
throw new Error('Element is not an HTMLInputElement');
100-
}
101-
return el.selectionEnd;
102-
});
91+
const selectionStart = await input.evaluate((/** @type {HTMLInputElement} */ el) => el.selectionStart);
92+
const selectionEnd = await input.evaluate((/** @type {HTMLInputElement} */ el) => el.selectionEnd);
10393
expect(selectionStart).toBe(startIndex);
10494
expect(selectionEnd).toBe(endIndex);
10595
}
@@ -109,12 +99,9 @@ export class OmnibarPage {
10999
*/
110100
async expectInputSelectionText(selectedText) {
111101
const input = this.searchInput();
112-
const selection = await input.evaluate((el) => {
113-
if (!(el instanceof HTMLInputElement)) {
114-
throw new Error('Element is not an HTMLInputElement');
115-
}
116-
return el.value.slice(el.selectionStart ?? 0, el.selectionEnd ?? 0);
117-
});
102+
const selection = await input.evaluate((/** @type {HTMLInputElement} */ el) =>
103+
el.value.slice(el.selectionStart ?? 0, el.selectionEnd ?? 0),
104+
);
118105
expect(selection).toBe(selectedText);
119106
}
120107

0 commit comments

Comments
 (0)