Merge pull request #86847 from NousResearch/salvage/72959-titlebar-tool-count
fix(desktop): count all five static titlebar buttons; find-bar overlay guard (salvage #72959)
This commit is contained in:
@@ -776,7 +776,10 @@ export function ContribController() {
|
||||
className="pointer-events-auto absolute z-10 flex w-max items-center gap-2 [-webkit-app-region:no-drag]"
|
||||
style={{
|
||||
right:
|
||||
'max(calc(var(--workspace-right, 0px) + 0.5rem), calc(var(--titlebar-tools-right, 0.75rem) + 4 * var(--titlebar-control-size, 24px) + 0.5rem))'
|
||||
// Five static cluster buttons: four systemTools plus the
|
||||
// always-present right-sidebar toggle (titlebar-controls.tsx).
|
||||
// Keep in sync with wiring.tsx's SYSTEM_TOOL_COUNT.
|
||||
'max(calc(var(--workspace-right, 0px) + 0.5rem), calc(var(--titlebar-tools-right, 0.75rem) + 5 * var(--titlebar-control-size, 24px) + 0.5rem))'
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -999,7 +999,12 @@ export function ContribWiring({ children }: { children: ReactNode }) {
|
||||
// Pane-registered tools (preview's monitor/devtools cluster) anchor flush
|
||||
// against the static system cluster — in the tree layout the titlebar band
|
||||
// sits ABOVE the grid, so AppShell's pane-width anchoring doesn't apply.
|
||||
const SYSTEM_TOOL_COUNT = 4
|
||||
// Count every button the static cluster actually renders: four systemTools
|
||||
// (layout, haptics, keybinds, settings) PLUS the always-present
|
||||
// right-sidebar toggle (see titlebar-controls.tsx). A shared width that
|
||||
// under-counts leaves the find bar, the titlebar header padding, and the
|
||||
// pane-cluster anchor overlapping the fifth button.
|
||||
const SYSTEM_TOOL_COUNT = 5
|
||||
const paneToolCount = rightTitlebarTools.filter(tool => !tool.hidden).length
|
||||
const systemToolsWidth = titlebarToolsWidthCss(SYSTEM_TOOL_COUNT)
|
||||
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { useNavigate } from 'react-router'
|
||||
import { useLocation, useNavigate } from 'react-router'
|
||||
|
||||
import { closeActiveTab } from '@/app/chat/close-tab'
|
||||
import { hudTargetSessionId } from '@/app/hud/handoff'
|
||||
import { setTerminalTakeover } from '@/app/right-sidebar/store'
|
||||
import { closeActiveTerminal, createTerminal, cycleTerminal } from '@/app/right-sidebar/terminal/terminals'
|
||||
import { appViewForPath, isOverlayView } from '@/app/routes'
|
||||
import {
|
||||
activateTreeTabSlot,
|
||||
cycleTreeTabInFocusedZone,
|
||||
@@ -98,6 +99,7 @@ type HandlerMap = Record<string, () => void>
|
||||
// mode is active (edit overlay / panel rebind) — records the pressed combo.
|
||||
export function useKeybinds(deps: KeybindRuntimeDeps): void {
|
||||
const navigate = useNavigate()
|
||||
const location = useLocation()
|
||||
const { resolvedMode, setMode } = useTheme()
|
||||
|
||||
// Keep the latest closures without re-subscribing the listener.
|
||||
@@ -256,7 +258,13 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
|
||||
// the Win/Linux path where ⌘W reaches the renderer directly.
|
||||
'view.closeTab': () => void closeActiveTab(id => navigate(sessionRoute(id))),
|
||||
'view.reopenTab': reopenLastClosedTile,
|
||||
'view.findInPage': openFindBar,
|
||||
'view.findInPage': () => {
|
||||
// Suppress on overlay routes so it doesn't collide with overlay-specific
|
||||
// search surfaces (e.g. Settings search bar).
|
||||
if (!isOverlayView(appViewForPath(location.pathname))) {
|
||||
openFindBar()
|
||||
}
|
||||
},
|
||||
// ⌘G / ⌘⇧G are handled by the find bar's own capture-phase listener while
|
||||
// it is open (so they don't collide with `view.toggleReview`). These
|
||||
// registry handlers cover a user-assigned dedicated chord: stepping is a
|
||||
|
||||
@@ -2,6 +2,7 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-libra
|
||||
import { MemoryRouter, useNavigate } from 'react-router'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { type KeybindRuntimeDeps, useKeybinds } from '@/app/hooks/use-keybinds'
|
||||
import { FindBar } from '@/components/find-bar'
|
||||
import { I18nProvider } from '@/i18n'
|
||||
import { en } from '@/i18n/en'
|
||||
@@ -22,6 +23,13 @@ import {
|
||||
updateFindResults
|
||||
} from '@/store/find-in-page'
|
||||
|
||||
// useKeybinds only needs the theme context for resolvedMode/setMode; the real
|
||||
// provider persists themes and subscribes to backend sync, which is unrelated
|
||||
// to the find-in-page gate under test.
|
||||
vi.mock('@/themes/context', () => ({
|
||||
useTheme: () => ({ resolvedMode: 'dark', setMode: vi.fn() })
|
||||
}))
|
||||
|
||||
// ── Bridge double ───────────────────────────────────────────────────────────
|
||||
// Stands in for the preload `hermesDesktop` surface. `onFoundInPage` records
|
||||
// its subscribers so the tests can assert the listener refcount and drive
|
||||
@@ -609,6 +617,69 @@ describe('FindBar', () => {
|
||||
await waitFor(() => expect(screen.queryByRole('search')).toBeNull())
|
||||
expect(bridge.stopFindInPage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not render on overlay routes (settings, command center, …)', () => {
|
||||
// Match isOverlayView: agents, command-center, cron, profiles, settings,
|
||||
// starmap, webhooks. Test with the most commonly hit one.
|
||||
openFindBar()
|
||||
renderFindBar('/settings')
|
||||
|
||||
expect(screen.queryByRole('search')).toBeNull()
|
||||
// The store still has active=true but the component guard suppresses
|
||||
// rendering — a harmless state ghost that the next pathname-change
|
||||
// cleanup will drain via closeFindBar.
|
||||
expect($findInPage.get().active).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
// ── Keybind gate: view.findInPage on overlay routes ─────────────────────────
|
||||
// The component guard above proves hidden RENDERING; this proves the keybind
|
||||
// itself never OPENS the bar on an overlay route. Mounted against the real
|
||||
// useKeybinds listener + combo index so a regression in either the handler
|
||||
// wiring or the route classification fails the test.
|
||||
|
||||
function KeybindHarness({ deps }: { deps: KeybindRuntimeDeps }) {
|
||||
useKeybinds(deps)
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
describe('view.findInPage keybind gate', () => {
|
||||
function renderKeybinds(pathname: string) {
|
||||
const deps: KeybindRuntimeDeps = {
|
||||
toggleCommandCenter: vi.fn(),
|
||||
startFreshSession: vi.fn(),
|
||||
openNewSessionTab: vi.fn(),
|
||||
toggleSelectedPin: vi.fn(),
|
||||
archiveSelectedSession: vi.fn()
|
||||
}
|
||||
|
||||
render(
|
||||
<MemoryRouter initialEntries={[pathname]}>
|
||||
<I18nProvider configClient={null} initialLocale="en">
|
||||
<KeybindHarness deps={deps} />
|
||||
</I18nProvider>
|
||||
</MemoryRouter>
|
||||
)
|
||||
|
||||
return deps
|
||||
}
|
||||
|
||||
it('does not open the find bar for mod+f while an overlay route is showing', () => {
|
||||
renderKeybinds('/settings')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'f', metaKey: true })
|
||||
|
||||
expect($findInPage.get().active).toBe(false)
|
||||
})
|
||||
|
||||
it('opens the find bar for mod+f on a normal chat route', () => {
|
||||
renderKeybinds('/session/a')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'f', metaKey: true })
|
||||
|
||||
expect($findInPage.get().active).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
// ── Files-pane-aware positioning ────────────────────────────────────────────
|
||||
|
||||
@@ -2,6 +2,7 @@ import { useStore } from '@nanostores/react'
|
||||
import { useEffect, useRef, useState } from 'react'
|
||||
import { useLocation } from 'react-router'
|
||||
|
||||
import { appViewForPath, isOverlayView } from '@/app/routes'
|
||||
import { Tip } from '@/components/ui/tooltip'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { findBarKeyAction, formatMatchLabel } from '@/lib/find-in-page'
|
||||
@@ -204,7 +205,7 @@ export function FindBar() {
|
||||
return () => window.removeEventListener('keydown', onKeyDown, { capture: true })
|
||||
}, [active])
|
||||
|
||||
if (!active) {
|
||||
if (!active || isOverlayView(appViewForPath(pathname))) {
|
||||
return null
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user