fix(browser): surface the inverted modifier on the hosted-review link

The hosted-review click path now passes modifierHeld, so with inverting on and
Link Routing off the chord opens in Orca — but the hint stayed gated on
openLinksInApp, hiding a live gesture. Resolve the destination instead of a
boolean. Default-off output is unchanged.
This commit is contained in:
Brennan Benson
2026-07-27 20:32:55 -07:00
parent 9b78a36f54
commit 6bcb73e64f
5 changed files with 131 additions and 18 deletions
@@ -2,6 +2,7 @@ import type { ReactNode } from 'react'
import { renderToStaticMarkup } from 'react-dom/server'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { ChecksPanelReviewHeader } from './ChecksPanel'
import type { ChecksPanelHostedReviewModifierDestination } from './checks-panel-hosted-review-click-routing'
vi.mock('@/components/ui/dropdown-menu', () => ({
DropdownMenu: ({ children }: { children: ReactNode }) => <div>{children}</div>,
@@ -30,11 +31,11 @@ afterEach(() => {
function renderHeader({
canUnlinkPullRequest = true,
provider = 'github',
showSystemBrowserHint = true
modifierHintDestination = 'system-browser'
}: {
canUnlinkPullRequest?: boolean
provider?: 'github' | 'gitlab'
showSystemBrowserHint?: boolean
modifierHintDestination?: ChecksPanelHostedReviewModifierDestination
} = {}): string {
const isGitLab = provider === 'gitlab'
return renderToStaticMarkup(
@@ -53,7 +54,7 @@ function renderHeader({
}}
isRefreshing={false}
canUnlinkPullRequest={canUnlinkPullRequest}
showSystemBrowserHint={showSystemBrowserHint}
modifierHintDestination={modifierHintDestination}
onRefresh={vi.fn()}
onOpenReview={vi.fn()}
onUnlinkPullRequest={vi.fn()}
@@ -79,14 +80,25 @@ describe('ChecksPanelReviewHeader', () => {
expect(markup).not.toContain('lucide-external-link')
})
it('omits the system-browser hint when plain clicks already open externally', () => {
const markup = renderHeader({ showSystemBrowserHint: false })
it('omits the modifier hint when it lands where a plain click already does', () => {
const markup = renderHeader({ modifierHintDestination: null })
expect(markup).toContain('Open on GitHub')
expect(markup).not.toContain('system browser')
expect(markup).not.toContain('⇧⌘+click')
})
// Why: with inverting on and Link Routing off the modifier reaches Orca here, so the
// hint must name Orca rather than the destination a plain click already uses.
it('names Orca when the modifier inverts toward the built-in browser', () => {
expect(renderHeader({ modifierHintDestination: 'orca' })).toContain('⇧⌘+click to open in Orca')
vi.stubGlobal('navigator', { userAgent: 'Windows' })
expect(renderHeader({ modifierHintDestination: 'orca' })).toContain(
'Shift+Ctrl+click to open in Orca'
)
})
it('shows the Ctrl system-browser hint off macOS', () => {
vi.stubGlobal('navigator', { userAgent: 'Windows' })
@@ -28,6 +28,7 @@ import { openHttpLink } from '@/lib/http-link-routing'
import { Button } from '@/components/ui/button'
import { DetachedHeadBadge } from '@/components/DetachedHeadBadge'
import {
getTerminalUrlOrcaBrowserHint,
getTerminalUrlSystemBrowserHint,
isMacPlatform
} from '../terminal-pane/terminal-link-open-hints'
@@ -171,7 +172,11 @@ import { stripBaseRef, useCreatePullRequestDialogFields } from './useCreatePullR
import { localizedHostedReviewCopy } from '@/i18n/hosted-review-localized-copy'
import { translate } from '@/i18n/i18n'
import { groupPRComments, type PRCommentGroup } from '@/lib/pr-comment-groups'
import { openChecksPanelHostedReviewUrl } from './checks-panel-hosted-review-click-routing'
import {
openChecksPanelHostedReviewUrl,
resolveChecksPanelHostedReviewModifierDestination,
type ChecksPanelHostedReviewModifierDestination
} from './checks-panel-hosted-review-click-routing'
import { ChecksPanelUpdatedAtMetadata } from './checks-panel-updated-at-metadata'
import {
clearPullRequestGenerationRequiresPushBeforeCreate,
@@ -250,7 +255,7 @@ type ChecksPanelReviewHeaderProps = {
review: ChecksPanelReview
isRefreshing: boolean
canUnlinkPullRequest: boolean
showSystemBrowserHint: boolean
modifierHintDestination: ChecksPanelHostedReviewModifierDestination
onRefresh: () => void
onOpenReview: (event: React.MouseEvent<HTMLButtonElement>) => void
onUnlinkPullRequest: () => void
@@ -261,7 +266,7 @@ export function ChecksPanelReviewHeader({
review,
isRefreshing,
canUnlinkPullRequest,
showSystemBrowserHint,
modifierHintDestination,
onRefresh,
onOpenReview,
onUnlinkPullRequest,
@@ -276,9 +281,13 @@ export function ChecksPanelReviewHeader({
'Open on {{value0}}',
{ value0: reviewHostLabel }
)
const title = showSystemBrowserHint
? `${openTitle}. ${getTerminalUrlSystemBrowserHint()}`
: openTitle
const modifierHint =
modifierHintDestination === 'system-browser'
? getTerminalUrlSystemBrowserHint()
: modifierHintDestination === 'orca'
? getTerminalUrlOrcaBrowserHint()
: null
const title = modifierHint ? `${openTitle}. ${modifierHint}` : openTitle
return (
<div className="flex items-center gap-2">
@@ -3997,11 +4006,10 @@ export default function ChecksPanel(): React.JSX.Element {
const reviewShortLabel = activeReview.provider === 'gitlab' ? 'MR' : 'PR'
const shouldShowReviewTriageStrip =
activeConflictReview !== null || getBrokenChecks(checks).length > 0
// Why: mirror openHttpLink's routing inputs so the hint only appears when a plain click would open inside Orca.
const showHostedReviewSystemBrowserHint =
Boolean(activeWorktreeId) &&
settings?.openLinksInApp === true &&
!settings.activeRuntimeEnvironmentId
const hostedReviewModifierHintDestination = resolveChecksPanelHostedReviewModifierDestination(
settings,
Boolean(activeWorktreeId)
)
return (
<div ref={setChecksPanelContentRef} className="flex-1 overflow-auto scrollbar-sleek">
{/* Why: surface a background-refresh failure over stale cached PR data so a GitHub outage doesn't look like a normal panel. GitHub-only. */}
@@ -4020,7 +4028,7 @@ export default function ChecksPanel(): React.JSX.Element {
review={activeReview}
isRefreshing={isRefreshing}
canUnlinkPullRequest={linkedPR !== null}
showSystemBrowserHint={showHostedReviewSystemBrowserHint}
modifierHintDestination={hostedReviewModifierHintDestination}
onRefresh={() => void handleRefresh()}
onOpenReview={handleOpenPR}
onUnlinkPullRequest={handleUnlinkPullRequest}
@@ -2,7 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
import {
isChecksPanelHostedReviewSystemBrowserModifier,
openChecksPanelHostedReviewUrl,
resolveChecksPanelHostedReviewHttpOpenOptions
resolveChecksPanelHostedReviewHttpOpenOptions,
resolveChecksPanelHostedReviewModifierDestination
} from './checks-panel-hosted-review-click-routing'
const { openHttpLinkMock } = vi.hoisted(() => ({ openHttpLinkMock: vi.fn() }))
@@ -73,3 +74,64 @@ describe('checks panel hosted review click routing', () => {
})
})
})
describe('checks panel hosted review modifier hint destination', () => {
it('names the system browser when a plain click already opens in Orca', () => {
expect(resolveChecksPanelHostedReviewModifierDestination({ openLinksInApp: true }, true)).toBe(
'system-browser'
)
})
// Why: this is the gesture the invert setting adds — without it the hint stays hidden
// and the only way to reach Orca from this button is undiscoverable.
it('names Orca when inverting is on and links open externally', () => {
expect(
resolveChecksPanelHostedReviewModifierDestination(
{ openLinksInApp: false, openLinksInAppModifierInverts: true },
true
)
).toBe('orca')
})
it('stays silent while inverting is off and links open externally', () => {
expect(
resolveChecksPanelHostedReviewModifierDestination({ openLinksInApp: false }, true)
).toBeNull()
expect(resolveChecksPanelHostedReviewModifierDestination(null, true)).toBeNull()
})
// Why: openHttpLink refuses to route a remote-owned link into Orca, and openLinksInApp
// cannot apply there either, so neither destination is reachable.
it('stays silent while a remote runtime is active', () => {
expect(
resolveChecksPanelHostedReviewModifierDestination(
{ openLinksInApp: true, activeRuntimeEnvironmentId: 'remote-1' },
true
)
).toBeNull()
expect(
resolveChecksPanelHostedReviewModifierDestination(
{
openLinksInApp: false,
openLinksInAppModifierInverts: true,
activeRuntimeEnvironmentId: 'remote-1'
},
true
)
).toBeNull()
})
// Why: openHttpLink gates routing to Orca on a worktree id, so without one the
// modifier lands in the system browser either way.
it('stays silent without a worktree', () => {
expect(
resolveChecksPanelHostedReviewModifierDestination(
{ openLinksInApp: false, openLinksInAppModifierInverts: true },
false
)
).toBeNull()
expect(
resolveChecksPanelHostedReviewModifierDestination({ openLinksInApp: true }, false)
).toBeNull()
})
})
@@ -22,6 +22,31 @@ export function resolveChecksPanelHostedReviewHttpOpenOptions(
return { worktreeId }
}
/** Where a Shift+modifier click lands, or null when it lands where a plain click already does. */
export type ChecksPanelHostedReviewModifierDestination = 'system-browser' | 'orca' | null
// Why: mirrors openHttpLink's routing inputs — with inverting on and Link Routing off the
// modifier now reaches Orca here, so gating the hint on openLinksInApp alone hides a live gesture.
export function resolveChecksPanelHostedReviewModifierDestination(
settings:
| {
openLinksInApp?: boolean
openLinksInAppModifierInverts?: boolean
activeRuntimeEnvironmentId?: string | null
}
| null
| undefined,
hasWorktree: boolean
): ChecksPanelHostedReviewModifierDestination {
if (!hasWorktree || settings?.activeRuntimeEnvironmentId) {
return null
}
if (settings?.openLinksInApp === true) {
return 'system-browser'
}
return settings?.openLinksInAppModifierInverts === true ? 'orca' : null
}
export function openChecksPanelHostedReviewUrl({
url,
event,
@@ -63,6 +63,12 @@ export function getTerminalUrlSystemBrowserHint(): string {
return isMacPlatform() ? '⇧⌘+click for system browser' : 'Shift+Ctrl+click for system browser'
}
// Why: the mirror of the system-browser hint for surfaces where inverting sends the
// modifier the other way; a plain click there already opens the system browser.
export function getTerminalUrlOrcaBrowserHint(): string {
return isMacPlatform() ? '⇧⌘+click to open in Orca' : 'Shift+Ctrl+click to open in Orca'
}
export function getTerminalWorktreePathOpenHint(canOpenWithSystemDefault: boolean): string {
if (!canOpenWithSystemDefault) {
return isMacPlatform() ? '⌘+click to switch workspace' : 'Ctrl+click to switch workspace'