Skip to content

Commit 5bb79fb

Browse files
NTP: Add tooltip above Customize button to highlight Omnibar feature (#1937)
* Add omnibar onboarding popover * Add tests for omnibar customize popover * Fix accidental focus ring appearing due to :focus-within capturing popover / tab switcher * Add screenshot test for omnibar customize popover * Fix: Use correct id for Popover description Co-authored-by: randerson <[email protected]> * Fix: Update Popover to use latest onClose ref Co-authored-by: randerson <[email protected]> * Fix Cursor overcomplicating things * Refactor useDrawerEventListeners to use refs and dependency array Co-authored-by: randerson <[email protected]> * Keep it simple Cursor... --------- Co-authored-by: Cursor Agent <[email protected]>
1 parent 9cdcc21 commit 5bb79fb

File tree

20 files changed

+398
-12
lines changed

20 files changed

+398
-12
lines changed

special-pages/pages/new-tab/app/components/Drawer.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,24 @@ function _close() {
157157
window.dispatchEvent(new CustomEvent(CLOSE_DRAWER_EVENT));
158158
}
159159

160+
/**
161+
* Hook for listening to drawer events
162+
* @param {object} callbacks
163+
* @param {() => void} [callbacks.onOpen] - Called when drawer opens
164+
* @param {() => void} [callbacks.onClose] - Called when drawer closes
165+
* @param {() => void} [callbacks.onToggle] - Called when drawer toggles
166+
* @param {any[]} [deps] - Dependency array controlling when listeners are re-registered
167+
*/
168+
export function useDrawerEventListeners({ onOpen, onClose, onToggle }, deps = []) {
169+
useEffect(() => {
170+
const controller = new AbortController();
171+
if (onOpen) window.addEventListener(OPEN_DRAWER_EVENT, onOpen, { signal: controller.signal });
172+
if (onClose) window.addEventListener(CLOSE_DRAWER_EVENT, onClose, { signal: controller.signal });
173+
if (onToggle) window.addEventListener(TOGGLE_DRAWER_EVENT, onToggle, { signal: controller.signal });
174+
return () => controller.abort();
175+
}, deps);
176+
}
177+
160178
/**
161179
* familiar React-style API
162180
*/
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { h } from 'preact';
2+
import { useEffect, useId, useRef } from 'preact/hooks';
3+
import { useTypedTranslationWith } from '../types.js';
4+
import { Cross } from './Icons.js';
5+
import styles from './Popover.module.css';
6+
7+
/**
8+
* @typedef {import('../strings.json')} Strings
9+
*/
10+
11+
/**
12+
* @param {object} props
13+
* @param {string} props.title
14+
* @param {() => void} props.onClose
15+
* @param {import('preact').ComponentChildren} props.children
16+
*/
17+
export function Popover({ title, onClose, children }) {
18+
const { t } = useTypedTranslationWith(/** @type {Strings} */ ({}));
19+
const titleId = useId();
20+
const descriptionId = useId();
21+
const popoverRef = useRef(/** @type {HTMLDivElement|null} */ (null));
22+
23+
useEffect(() => {
24+
popoverRef.current?.focus();
25+
26+
/** @type {(event: KeyboardEvent) => void} */
27+
const handleEscapeKey = (event) => {
28+
if (event.key === 'Escape') {
29+
onClose();
30+
}
31+
};
32+
33+
document.addEventListener('keydown', handleEscapeKey);
34+
return () => document.removeEventListener('keydown', handleEscapeKey);
35+
}, [onClose]);
36+
37+
return (
38+
<div ref={popoverRef} class={styles.popover} role="dialog" aria-labelledby={titleId} aria-describedby={descriptionId} tabIndex={-1}>
39+
<svg class={styles.arrow} xmlns="http://www.w3.org/2000/svg" width="12" height="30" viewBox="0 0 12 30" fill="none">
40+
<path
41+
d="M9.20362 6.3927L0.510957 13.8636C-0.183621 14.4619 -0.16344 15.5367 0.531137 16.1351L9.20362 23.606C10.9677 25.1256 11.9819 27.3368 11.9819 29.6632L11.9819 30.0003L11.9819 -0.000488281V0.335449C11.9819 2.66185 10.9677 4.87302 9.20362 6.3927Z"
42+
fill="currentColor"
43+
/>
44+
</svg>
45+
<div class={styles.content}>
46+
<button class={styles.closeButton} onClick={onClose} aria-label={t('ntp_popover_close_button')}>
47+
<Cross />
48+
</button>
49+
<h3 id={titleId} class={styles.title}>
50+
{title}
51+
</h3>
52+
<p id={descriptionId} class={styles.description}>
53+
{children}
54+
</p>
55+
</div>
56+
</div>
57+
);
58+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
.popover {
2+
align-items: center;
3+
display: flex;
4+
filter: drop-shadow(0 0 calc(0.5px) rgba(0, 0, 0, 0.75))
5+
drop-shadow(0 0 calc(1px) rgba(0, 0, 0, 0.25))
6+
drop-shadow(0 calc(8px) calc(16px) rgba(0, 0, 0, 0.20));
7+
flex-direction: row;
8+
left: calc(100% + 7px);
9+
outline: none;
10+
position: absolute;
11+
top: 50%;
12+
transform: translateY(-50%);
13+
width: 390px;
14+
z-index: 1000;
15+
}
16+
17+
.content {
18+
background: var(--ntp-surface-tertiary);
19+
border-radius: 16px;
20+
flex: auto;
21+
padding: var(--sp-4) var(--sp-11) var(--sp-4) var(--sp-5);
22+
position: relative;
23+
}
24+
25+
.closeButton {
26+
background: none;
27+
border-radius: 100%;
28+
border: none;
29+
color: var(--ntp-icons-primary);
30+
cursor: pointer;
31+
padding: 0;
32+
position: absolute;
33+
right: var(--sp-4);
34+
top: var(--sp-4);
35+
36+
&:hover {
37+
background: rgba(0, 0, 0, 0.06); /* @todo: dark mode? */
38+
}
39+
40+
&:active {
41+
background: rgba(0, 0, 0, 0.12); /* @todo: dark mode? */
42+
}
43+
44+
&:focus-visible {
45+
box-shadow: var(--focus-ring);
46+
}
47+
}
48+
49+
.title {
50+
font-size: var(--title-3-em-font-size);
51+
font-weight: var(--title-3-em-font-weight);
52+
line-height: var(--title-3-em-line-height);
53+
}
54+
55+
.description {
56+
button {
57+
background: none;
58+
border: none;
59+
color: var(--ntp-accent-primary);
60+
cursor: pointer;
61+
padding: 0;
62+
63+
&:hover {
64+
color: var(--ntp-accent-secondary);
65+
}
66+
67+
&:active {
68+
color: var(--ntp-accent-tertiary);
69+
}
70+
}
71+
}
72+
73+
.arrow {
74+
color: var(--ntp-surface-tertiary);
75+
margin-right: -1px;
76+
}

special-pages/pages/new-tab/app/customizer/components/CustomizerMenu.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export function CustomizerButton({ menuId, buttonId, isOpen, toggleMenu, buttonR
8585
aria-controls={menuId}
8686
data-kind={kind}
8787
id={buttonId}
88+
data-testid="customizer-button"
8889
>
8990
<CustomizeIcon />
9091
<span>{t('ntp_customizer_button')}</span>

special-pages/pages/new-tab/app/omnibar/components/Omnibar.js

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { h } from 'preact';
2-
import { useContext, useState } from 'preact/hooks';
2+
import { useCallback, useContext, useState } from 'preact/hooks';
33
import { LogoStacked } from '../../components/Icons';
44
import { useTypedTranslationWith } from '../../types';
55
import { AiChatForm } from './AiChatForm';
@@ -11,6 +11,9 @@ import { SearchFormProvider } from './SearchFormProvider';
1111
import { SuggestionsList } from './SuggestionsList';
1212
import { TabSwitcher } from './TabSwitcher';
1313
import { useQueryWithLocalPersistence } from './PersistentOmnibarValuesProvider.js';
14+
import { Popover } from '../../components/Popover';
15+
import { useDrawerControls, useDrawerEventListeners } from '../../components/Drawer';
16+
import { Trans } from '../../../../../shared/components/TranslationsProvider.js';
1417

1518
/**
1619
* @typedef {import('../strings.json')} Strings
@@ -24,22 +27,36 @@ import { useQueryWithLocalPersistence } from './PersistentOmnibarValuesProvider.
2427
* @param {OmnibarConfig['mode']} props.mode
2528
* @param {(mode: OmnibarConfig['mode']) => void} props.setMode
2629
* @param {boolean} props.enableAi
30+
* @param {boolean} props.showCustomizePopover
2731
* @param {string|null|undefined} props.tabId
2832
*/
29-
export function Omnibar({ mode, setMode, enableAi, tabId }) {
33+
export function Omnibar({ mode, setMode, enableAi, showCustomizePopover, tabId }) {
3034
const { t } = useTypedTranslationWith(/** @type {Strings} */ ({}));
3135

3236
const [query, setQuery] = useQueryWithLocalPersistence(tabId);
3337
const [resetKey, setResetKey] = useState(0);
3438
const [autoFocus, setAutoFocus] = useState(false);
3539

36-
const { openSuggestion, submitSearch, submitChat } = useContext(OmnibarContext);
40+
const { openSuggestion, submitSearch, submitChat, setShowCustomizePopover } = useContext(OmnibarContext);
41+
42+
const { open: openCustomizer } = useDrawerControls();
43+
useDrawerEventListeners(
44+
{
45+
onOpen: () => setShowCustomizePopover(false),
46+
onToggle: () => setShowCustomizePopover(false),
47+
},
48+
[setShowCustomizePopover],
49+
);
3750

3851
const resetForm = () => {
3952
setQuery('');
4053
setResetKey((prev) => prev + 1);
4154
};
4255

56+
const handleCloseCustomizePopover = useCallback(() => {
57+
setShowCustomizePopover(false);
58+
}, [setShowCustomizePopover]);
59+
4360
/** @type {(params: {suggestion: Suggestion, target: OpenTarget}) => void} */
4461
const handleOpenSuggestion = (params) => {
4562
openSuggestion(params);
@@ -67,7 +84,23 @@ export function Omnibar({ mode, setMode, enableAi, tabId }) {
6784
return (
6885
<div key={resetKey} class={styles.root} data-mode={mode}>
6986
<LogoStacked class={styles.logo} aria-label={t('omnibar_logoAlt')} />
70-
{enableAi && <TabSwitcher mode={mode} onChange={handleChangeMode} />}
87+
{enableAi && (
88+
<div class={styles.tabSwitcherContainer}>
89+
<TabSwitcher mode={mode} onChange={handleChangeMode} />
90+
{showCustomizePopover && (
91+
<Popover title={t('omnibar_customizePopoverTitle')} onClose={handleCloseCustomizePopover}>
92+
<Trans
93+
str={t('omnibar_customizePopoverDescription')}
94+
values={{
95+
button: {
96+
click: () => openCustomizer(),
97+
},
98+
}}
99+
/>
100+
</Popover>
101+
)}
102+
</div>
103+
)}
71104
<SearchFormProvider term={query} setTerm={setQuery}>
72105
<div class={styles.spacer}>
73106
<div class={styles.popup}>

special-pages/pages/new-tab/app/omnibar/components/Omnibar.module.css

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
width: 164px;
2020
}
2121

22+
.tabSwitcherContainer {
23+
position: relative;
24+
}
25+
2226
/*
2327
* There are three containers:
2428
* 1) .spacer - Pushes other widgets away. Fixed height to accommodate largest tab (Duck.ai). Contents overflow. No animation.
@@ -57,12 +61,12 @@
5761
}
5862

5963
/* Typing */
60-
.root:focus-within .popup {
64+
.root:has(input:focus, textarea:focus) .popup {
6165
box-shadow: 0 4px 12px 0 rgba(0, 0, 0, 0.1), 0 20px 40px 0 rgba(0, 0, 0, 0.08); /* Elevation 90 */
6266
}
6367

6468
/* Focused */
65-
.root:focus-within:has(input:placeholder-shown, textarea:placeholder-shown) .popup {
69+
.root:has(input:focus:placeholder-shown, textarea:focus:placeholder-shown) .popup {
6670
border-radius: 15px;
6771
border: 2px solid var(--ntp-accent-primary);
6872
box-shadow: 0 4px 12px 0 rgba(0, 0, 0, 0.1), 0 20px 40px 0 rgba(0, 0, 0, 0.08); /* Elevation 90 */

special-pages/pages/new-tab/app/omnibar/components/OmnibarConsumer.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,19 @@ export function OmnibarConsumer() {
5050
* @param {string} props.tabId
5151
*/
5252
function OmnibarReadyState({ config, tabId }) {
53-
const { enableAi = true, showAiSetting = true, mode: defaultMode } = config;
53+
const { enableAi = true, showAiSetting = true, showCustomizePopover = false, mode: defaultMode } = config;
5454
const { setMode } = useContext(OmnibarContext);
5555
const modeForCurrentTab = useModeWithLocalPersistence(tabId, defaultMode);
5656

57-
return <Omnibar mode={modeForCurrentTab} setMode={setMode} enableAi={showAiSetting && enableAi} tabId={tabId} />;
57+
return (
58+
<Omnibar
59+
mode={modeForCurrentTab}
60+
setMode={setMode}
61+
enableAi={showAiSetting && enableAi}
62+
showCustomizePopover={showCustomizePopover}
63+
tabId={tabId}
64+
/>
65+
);
5866
}
5967

6068
/**

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ export const OmnibarContext = createContext({
2626
setEnableAi: () => {
2727
throw new Error('must implement');
2828
},
29+
/** @type {(showCustomizePopover: NonNullable<OmnibarConfig['showCustomizePopover']>) => void} */
30+
setShowCustomizePopover: () => {
31+
throw new Error('must implement');
32+
},
2933
/** @type {(term: string) => Promise<SuggestionsData>} */
3034
getSuggestions: () => {
3135
throw new Error('must implement');
@@ -90,6 +94,14 @@ export function OmnibarProvider(props) {
9094
[service],
9195
);
9296

97+
/** @type {(showCustomizePopover: NonNullable<OmnibarConfig['showCustomizePopover']>) => void} */
98+
const setShowCustomizePopover = useCallback(
99+
(showCustomizePopover) => {
100+
service.current?.setShowCustomizePopover(showCustomizePopover);
101+
},
102+
[service],
103+
);
104+
93105
/** @type {(term: string) => Promise<SuggestionsData>} */
94106
const getSuggestions = useCallback(
95107
(term) => {
@@ -138,6 +150,7 @@ export function OmnibarProvider(props) {
138150
state,
139151
setMode,
140152
setEnableAi,
153+
setShowCustomizePopover,
141154
getSuggestions,
142155
onSuggestions,
143156
openSuggestion,

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class OmnibarPage {
6060
}
6161

6262
customizeButton() {
63-
return this.page.getByRole('button', { name: 'Customize' });
63+
return this.page.getByTestId('customizer-button');
6464
}
6565

6666
toggleSearchButton() {
@@ -75,6 +75,18 @@ export class OmnibarPage {
7575
return this.context().getByRole('button', { name: 'Close' });
7676
}
7777

78+
popover() {
79+
return this.context().getByRole('dialog');
80+
}
81+
82+
popoverCloseButton() {
83+
return this.popover().getByRole('button', { name: 'Close' });
84+
}
85+
86+
popoverCustomizeButton() {
87+
return this.popover().getByRole('button', { name: 'Customize' });
88+
}
89+
7890
root() {
7991
return this.context().locator('[data-mode]');
8092
}

0 commit comments

Comments
 (0)