You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have tested this change locally in all supported browsers
This change will be visible to users
I have added automated tests that cover this change
I have ensured the change is gated by config
This change was covered by a ship review
This change was covered by a tech design
Any dependent config has been merged
Note
Medium Risk
New native messaging and config drive AI response customization UX; changes are config-gated but depend on correct native handling of open/toggle notifies.
Overview
Adds a Customize Responses row to the NTP AI chat Tools menu when enableCustomizeResponses is on, gated by omnibar config (customizeSubLabel, hasCustomization, customizationActive).
Menu construction moves into useToolsMenu / useCustomizeResponsesItem: the row opens the native modal via omnibar_openCustomizeResponses, and when the user has saved settings it shows an inline Switch that calls omnibar_setCustomizeResponsesActive. The Tools trigger collapses to icon-only when a tool chip is active or customization is applied; the row is disabled during image generation and separated from tool checkboxes when both are shown.
Shared Dropdown support grows for this pattern: separators, disabled rows, trailing controls (toggle clicks don’t fire the row), and keyboard navigation that skips non-actionable items.
Reviewed by Cursor Bugbot for commit 5b0be20. Bugbot is set up for automated code reviews on this repo. Configure here.
Bring the New Tab Page omnibar Tools-menu "Customize responses" row to
parity with the address bar: show the customization summary as the row
sub-heading and an on/off toggle once responses have been customized.
- omnibar-config: add hasCustomization + customizationActive
- new message omnibar_setCustomizeResponsesActive to apply/unapply the
stored customization
- DropdownItem gains an interactive trailingControl slot; ToolsMenu
renders the shared Switch bound to the active state
- service/provider wiring, strings, mock-transport support
Overall it works as expected. One thing I'd suggest changing is how we render the "Customize responses" dropdown item.
Right now we pass a batch of props specific to customize responses to ToolsMenu (props): showCustomizeResponses, customizeResponsesSubLabel, hasCustomization, customizeResponsesActive, onSetCustomizeResponsesActive, and onOpenCustomizeResponses. These get threaded down through OmnibarConsumer → Omnibar → AiChatContent → ToolsMenu. That doesn't scale: every new menu entry of this kind adds another batch of props through the same three layers, and ToolsMenu's API keeps growing.
Worth noting we already avoid this for tools. useActiveTools reads its config straight from OmnibarContext, so ToolsMenu only takes tools, activeTool, and onToggle. Customize responses is the odd one out.
I'd refactor it so the config comes from OmnibarContext instead of props, the way useActiveTools already feeds the tools. Concretely: a useCustomizeResponses hook for the state and actions, and the row resolved to a uniform config rendered from the same .map the tools use. ToolsMenu then drops the six props and stays a 3-prop shell, so the next menu entry is another config source instead of more props and JSX.
I've prepared a commit that should give you some ideas: 0a0982b.
The "Customize responses" row was passed into ToolsMenu as six
feature-specific props threaded through OmnibarConsumer, Omnibar, and
AiChatContent, which does not scale as more menu entries are added.
Read the customize config and actions from OmnibarContext via a
useCustomizeResponses hook, mirroring how useActiveTools feeds the
tools, and render every row (tools and customize) from one uniform
config list. Extract the customize row into useCustomizeResponsesItem
so the feature stays self-contained and ToolsMenu keeps a three-prop
surface (tools, activeTool, onToggle).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DropdownSeparator children return {} from getItemProps() (not null), so separators are included in navigableIndices. Arrow/Home/End can land on a separator; aria-activedescendant may reference an id injected via cloneElement that DropdownSeparator does not forward to its <li>, producing a phantom focus target. Filter separators out of getNavigableIndices() (e.g. treat role="separator" or return null from getItemProps for non-item children).
Keyboard Space/Enter on the customize-responses row invokes onSelect (opens modal) via Dropdown.handleKeyDown; the embedded Switch only receives mouse clicks (stopPropagation on the trailing span). Keyboard users cannot toggle the switch without Tab-stopping into it. Consider role="menuitemcheckbox" with row-level toggle behavior, or intercept Space when focus is on a row with trailingControl.
Disabled rows (e.g. customize-responses when image-generation is active) remain in navigableIndices; keyboard can highlight them but selectAt no-ops. Acceptable given aria-disabled + pointer-events: none, but skipping disabled indices would be cleaner.
All special-pages/ changes
—
info
No injected/ changes. This code runs in the embedded first-party NTP, not in third-party page context — no browser API fidelity, prototype-chain, or third-party script detection concerns apply.
Schema lacks additionalProperties: false (consistent with most omnibar notifies like omnibar_viewAllAIChats; only omnibar_submitChat enforces it). Low risk given empty fixed payload.
customizeSubLabel from native config is rendered as Preact text content (auto-escaped). No dangerouslySetInnerHTML.
Risk Level
Low Risk — special-pages NTP omnibar UI only; no injected runtime, API overrides, messaging transport changes, or security-boundary modifications.
Recommendations
(warning) Exclude DropdownSeparator from getNavigableIndices() and ensure separator <li> elements are not aria-activedescendant targets.
(warning) Align keyboard behavior for the customize-responses toggle with mouse behavior (row Space/Enter toggles when hasCustomization, or document Tab-to-switch as intentional).
(info) Add integration tests for the customize-responses row (visibility gating, toggle notify, disabled-when-image-gen, separator placement) — mock transport already handles both notifies.
(info) Consider additionalProperties: false on new notify schemas for consistency with stricter messages.
Synchronize note (cab5a3b)
Latest commit refactors customize-responses into useCustomizeResponses / useCustomizeResponsesItem hooks (mirrors useActiveTools pattern), removing prop drilling through Omnibar → ToolsMenu. Separator now renders only when index > 0. No new compatibility or security surface introduced by the refactor.
Wire up omnibar.enableCustomizeResponses, omnibar.customizeSubLabel,
omnibar.hasCustomization, and omnibar.customizationActive in the new-tab
mock transport, matching the existing omnibar.* overrides, so the
Customize responses row and its states can be exercised from the URL in
local dev.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Skip separators and disabled rows in Dropdown keyboard navigation so
arrow/Home/End never land on a non-actionable element or point
aria-activedescendant at a separator.
- Drop native notify method names from omnibar config type descriptions.
- Add additionalProperties: false to the new customize-responses notify
schemas, matching stricter omnibar messages.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation — Web Compatibility & Security
Re-assessed on synchronize (0fdc9c75f). Latest commit addresses prior review feedback (separator keyboard nav, additionalProperties: false on notify schemas).
Web Compatibility Assessment
File
Lines
Severity
Finding
(none — injected/)
—
—
No changes to injected runtime code. This PR is confined to special-pages/pages/new-tab/ (first-party NTP omnibar UI). Standard web-compat categories (API overrides, prototype patching, third-party script detection) do not apply.
Dropdown.js
71–77, 143–154
info
getNavigableIndices() correctly skips DropdownSeparator and disabled rows — prior separator keyboard-nav concern is resolved in 0fdc9c75f.
Dropdown.js + useCustomizeResponsesItem.js
143–154 / 64
warning
When the Customize Responses row is keyboard-focused, Space/Enter invoke onSelect (open modal) rather than toggling the embedded Switch. Mouse users can click the switch (stopPropagation on trailing control), but keyboard-only users cannot toggle without opening the modal first. Consider intercepting Space when focus is on the trailing control, or making the switch independently focusable within the menu.
useCustomizeResponsesItem.js
42, 50
info
Row is disabled when activeTool === 'image-generation' — sensible mutual-exclusion with image-gen mode. pointer-events: none on disabled rows also blocks mouse access to the switch.
ToolsMenu.js
486–488
info
Separator inserted only when customize-responses is not the first item — correct grouping UX.
(tests)
—
info
No dedicated integration tests for the customize-responses Tools menu row or toggle notify. Mock transport handles both new notifies; consider Playwright coverage for keyboard nav + toggle.
Security Assessment
File
Lines
Severity
Finding
(none — injected/)
—
—
No changes to captured-globals.js, message bridge, wrapper utilities, or injected features.
useCustomizeResponses.js
605
info
Feature visibility gated on enableCustomizeResponses === true (schema default false) — supports remote rollback without deploy.
omnibar.service.js
666–675
info
New notifies use explicit, minimal payloads: omnibar_openCustomizeResponses → {}; omnibar_setCustomizeResponsesActive → { active: boolean }. No object spreading; no nativeData leakage risk.
customizeSubLabel from native config rendered as Preact text node — auto-escaped; no HTML injection vector.
Risk Level
Low Risk — Special-pages-only UI change with config-gated visibility, closed notify schemas, and no injected/API-override/messaging-transport modifications.
Recommendations
(warning) Improve keyboard a11y for the embedded Switch: either focus-trap into the trailing control on ArrowRight, or suppress row-level Space/Enter when focus is within the switch.
(info) Add integration tests covering: customize-responses row visibility (enableCustomizeResponses), toggle notify (omnibar_setCustomizeResponsesActive), separator keyboard nav, and disabled state during image-generation.
(info) Confirm native platforms handle both new notifies before shipping (expected coordination).
Move the menu-building logic out of ToolsMenu into a useToolsMenuItems
hook that composes all menu sources (tools + Customize responses) into a
single view model: { items, activeItem, isCollapsed }. ToolsMenu becomes
render-only, and Omnibar gates the menu on items.length instead of
checking availableTools and showCustomizeResponses separately, making the
data flow explicit and scalable as more menu entries are added.
Add a separatorBefore flag to the menu item config and set it on the
"Customize responses" row when tool rows precede it, so ToolsMenu renders
the divider from config instead of a customize-responses-specific
index check.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
PR: Add Customize Responses to NTP omnibar Tools menu (ff7dbb2a) Scope:special-pages/pages/new-tab/ only — no injected/, messaging/, or captured-globals changes. This UI runs in the first-party NTP embedded page, not in third-party page injection.
Web Compatibility Assessment
File
Lines
Severity
Finding
(none in injected scope)
—
info
No browser API overrides, prototype patches, or DOM patterns that affect third-party sites.
dropdown/Dropdown.js
149–160
warning
Keyboard vs trailing toggle:Space/Enter on the focused row invoke onSelect (opens Customize Responses modal) rather than toggling the embedded Switch. Mouse users can click the switch (stopPropagation on trailingControl), but keyboard-only users cannot operate the toggle from the menu.
dropdown/Dropdown.js
165–167
warning
Tab closes menu:Tab schedules onClose({ restoreFocus: false }), so focus cannot move into the trailing Switch while the menu stays open.
dropdown/Dropdown.js
71–111
info
Positive:getNavigableIndices() / focusNavigable('up'|'down') correctly skip DropdownSeparator rows and disabled items; aria-activedescendant won't point at separators.
tools-menu/useCustomizeResponsesItem.js
42, 49
info
Customize Responses row is disabled when image-generation is active — avoids conflicting tool states.
tools-menu/useToolsMenu.js
74
info
Tools button collapses to icon-only when a tool chip or active customization is shown — localized NTP layout change only.
omnibar/mocks/omnibar.mock-transport.js
203–208
warning
Missing break:omnibar_setCustomizeResponsesActive falls through into omnibar_removeSuggestion, logging msg.params.url as undefined in local dev mocks.
integration-tests/
—
info
No dedicated integration tests for the Customize Responses row or its notifies; existing tools-menu tests cover Create Image / Web Search only.
Security Assessment
File
Lines
Severity
Finding
(scope)
—
info
Changes are confined to the NTP special page. Standard injected threat model (prototype tampering, captured-globals, message-bridge spoofing) does not apply.
omnibar.service.js
218–227
info
Explicit notify payloads:notify('omnibar_openCustomizeResponses', {}) and notify('omnibar_setCustomizeResponsesActive', { active }) — no object spreading; no nativeData leakage risk.
messages/omnibar_*.notify.json
—
info
Both schemas set additionalProperties: false; toggle schema requires boolean active.
customizeSubLabel from native config is rendered as Preact text (description prop) — auto-escaped; no dangerouslySetInnerHTML.
messages/types/omnibar-config.json
189–212
info
New config fields are typed booleans/strings with defaults; no executable config surface.
Risk Level
Low Risk — Special-pages NTP UI only; no injected runtime, API shims, or messaging transport changes; feature is config-gated with tightly scoped notify schemas.
Recommendations
(warning) Add break; after the omnibar_setCustomizeResponsesActive case in omnibar.mock-transport.js to prevent fall-through.
(warning) Consider keyboard accessibility for the trailing Switch: e.g. don't preventDefault Space when focus is inside trailingControl, or expose a separate toggle action via keyboard.
(info) Add integration tests asserting omnibar_openCustomizeResponses / omnibar_setCustomizeResponsesActive notifies when the row is selected and the toggle is clicked.
(info) Coordinate native handler rollout for the two new notify methods before enabling enableCustomizeResponses in production config.
Space/Enter on the highlighted row always invokes onSelect (opens Customize Responses modal). When trailingControl embeds a Switch, keyboard users cannot toggle active state from the row — only mouse clicks on the switch work (stopPropagation on click, not keydown). Consider forwarding Space/Enter to the focused trailing control or using a roving-tabindex pattern.
Tab closes the menu (onClose({ restoreFocus: false })) before focus can move into the embedded Switch, so keyboard users cannot reach the toggle without the menu closing first.
Row is disabled when image-generation is active (pointer-events: none via .itemDisabled). Consistent with product intent; disabled row is excluded from getNavigableIndices().
getNavigableIndices() / focusNavigable('up'/'down') correctly skip separators and disabled rows — avoids aria-activedescendant pointing at non-actionable elements.
(no injected/ changes)
—
info
No browser API overrides, prototype patches, or third-party page interaction. Web-compat evaluation categories (API fidelity, prototype chain, DOM on hostile pages) do not apply.
customizeSubLabel from native config is rendered as Preact text content ({description} in DropdownItem) — auto-escaped, no dangerouslySetInnerHTML. Trust boundary is native→NTP (first-party).
Fixed in 5b0be2087:omnibar_setCustomizeResponsesActive case now has break; — no longer falls through to omnibar_removeSuggestion.
(no injected/ changes)
—
info
No changes to captured-globals.js, message-bridge trust boundary, origin validation, or postMessage usage.
Risk Level
Low Risk — Changes are confined to the first-party NTP special-page (special-pages/pages/new-tab/); no injected content-script code, API shims, or messaging transport changes that affect third-party web pages.
Recommendations
(warning) Improve keyboard a11y for the embedded Switch: either handle Space/Enter at the Dropdown level when trailingControl is present, or allow Tab to move focus into the switch without closing the menu.
(info) Add an integration test for the customize-responses row (toggle visibility, disabled state during image generation, omnibar_openCustomizeResponses / omnibar_setCustomizeResponsesActive notifies) — mock transport and query-param overrides are already wired.
(info) Coordinate native handling for the two new notifies and four config fields before enabling enableCustomizeResponses in production.
Positive notes
Config-gated with safe defaults (enableCustomizeResponses: false).
Message schemas use additionalProperties: false and explicit payloads.
Dropdown keyboard-nav refactor (getNavigableIndices, separator support, disabled rows) is a solid improvement shared by all Tools-menu rows.
Mock transport fallthrough from prior review is resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
semver-minorNew feature — triggers minor version bump
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/72649045549333/task/1215104074883750?focus=true
Description
Adds the Customize Responses entry to the NTP AI chat Tools menu, wired through omnibar config and native messaging.
enableCustomizeResponses,customizeSubLabel,hasCustomization,customizationActiveomnibar_openCustomizeResponses,omnibar_setCustomizeResponsesActivehasCustomizationis truecustomizationActive && hasCustomization)Testing Steps
special-pagesnpm run watch -- --page=new-tabCustomize Responseitem renders.Checklist
Please tick all that apply:
Note
Medium Risk
New native messaging and config drive AI response customization UX; changes are config-gated but depend on correct native handling of open/toggle notifies.
Overview
Adds a Customize Responses row to the NTP AI chat Tools menu when
enableCustomizeResponsesis on, gated by omnibar config (customizeSubLabel,hasCustomization,customizationActive).Menu construction moves into
useToolsMenu/useCustomizeResponsesItem: the row opens the native modal viaomnibar_openCustomizeResponses, and when the user has saved settings it shows an inline Switch that callsomnibar_setCustomizeResponsesActive. The Tools trigger collapses to icon-only when a tool chip is active or customization is applied; the row is disabled during image generation and separated from tool checkboxes when both are shown.Shared Dropdown support grows for this pattern: separators, disabled rows, trailing controls (toggle clicks don’t fire the row), and keyboard navigation that skips non-actionable items.
Reviewed by Cursor Bugbot for commit 5b0be20. Bugbot is set up for automated code reviews on this repo. Configure here.