mirror of
https://github.com/wu736139669/hapi.git
synced 2026-11-04 00:01:41 +00:00
feat(web): setting to open external links in a new tab (#1884)
* feat(web): setting to open external links in a new tab Left-clicking an external http(s) link renders it with no explicit target, defaulting to _self. In an installed/standalone PWA this gets intercepted by the browser and shown as a captive in-app overlay instead of leaving the app; in a normal tab it navigates away from HAPI entirely. Right-click already opens in a new tab, but that's not a fair ask for every link. Add an opt-in Settings > Display > Links toggle that forces target="_blank" rel="noopener noreferrer" on genuine external http(s) links only (relative/SPA hrefs and other schemes like mailto are left untouched). Defaults to off, preserving today's behaviour, since the overlay may be intentional/liked by some users. Vibe Coding disclosure: implemented with Claude Sonnet 5 (Claude Code). * fix(web): simplify link-toggle setting description per review The description mentioned the target attribute (an implementation detail, not user-facing) and asserted the in-app overlay is what every installed PWA shows, which was only actually confirmed on Chromium/ Edge. Describe the user-visible behaviour only, without promising a specific rendering across all installed-app environments. * fix(web): stop promising specific behaviour when installed as an app Per follow-up review: describing the setting as "always" opening links in a new tab "even when installed as an app" still guarantees a specific outcome across every browser's PWA handling, which isn't something this change can verify or promise. State only what the setting does (open in a separate tab), not how every installed-app environment will render that. --------- Co-authored-by: Ian Stevenson <is@lockhouse.systems>
This commit is contained in:
co-authored by
Ian Stevenson
parent
a7ad631691
commit
773af2cf65
@@ -66,6 +66,7 @@ function chatContext(overrides: Partial<HappyChatContextValue> = {}): HappyChatC
|
||||
}
|
||||
|
||||
const STORAGE_KEY = 'hapi-allowed-schemes'
|
||||
const OPEN_NEW_TAB_STORAGE_KEY = 'hapi-open-external-links-in-new-tab'
|
||||
|
||||
beforeEach(() => {
|
||||
localStorage.clear()
|
||||
@@ -422,6 +423,51 @@ describe('markdown <A> component — fail-closed path-like hrefs (#1452)', () =>
|
||||
})
|
||||
})
|
||||
|
||||
// ── "open external links in a new tab" setting (Settings > Display > Links) ──
|
||||
|
||||
describe('markdown <A> component — open external links in new tab setting', () => {
|
||||
it('defaults to no explicit target on an external http(s) link', () => {
|
||||
renderA({ href: 'https://example.com', children: 'link' })
|
||||
expect(document.querySelector('a')!.getAttribute('target')).toBeNull()
|
||||
})
|
||||
|
||||
it('forces target="_blank" + rel="noopener noreferrer" when the setting is enabled', () => {
|
||||
localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true')
|
||||
renderA({ href: 'https://example.com', children: 'link' })
|
||||
const link = document.querySelector('a')!
|
||||
expect(link.getAttribute('target')).toBe('_blank')
|
||||
expect(link.getAttribute('rel')).toBe('noopener noreferrer')
|
||||
})
|
||||
|
||||
it('does not force target on a relative/SPA href even when the setting is enabled', () => {
|
||||
localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true')
|
||||
renderA({ href: '/settings', children: 'settings' }, chatContext())
|
||||
expect(document.querySelector('a')!.getAttribute('target')).toBeNull()
|
||||
})
|
||||
|
||||
it('does not force target on a non-http(s) IANA scheme (mailto) even when enabled', () => {
|
||||
localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true')
|
||||
renderA({ href: 'mailto:a@b.com', children: 'mail' })
|
||||
expect(document.querySelector('a')!.getAttribute('target')).toBeNull()
|
||||
})
|
||||
|
||||
it('updates an already-rendered link when the setting changes via storage event (settings page in another tab, or Settings > Display in this one after context refactor)', () => {
|
||||
renderA({ href: 'https://example.com', children: 'link' })
|
||||
expect(document.querySelector('a')!.getAttribute('target')).toBeNull()
|
||||
|
||||
act(() => {
|
||||
localStorage.setItem(OPEN_NEW_TAB_STORAGE_KEY, 'true')
|
||||
window.dispatchEvent(new StorageEvent('storage', {
|
||||
key: OPEN_NEW_TAB_STORAGE_KEY,
|
||||
newValue: 'true',
|
||||
storageArea: localStorage,
|
||||
}))
|
||||
})
|
||||
|
||||
expect(document.querySelector('a')!.getAttribute('target')).toBe('_blank')
|
||||
})
|
||||
})
|
||||
|
||||
// ── intra-tab cross-provider sync (schemeListeners emitter) ──────────────────
|
||||
//
|
||||
// P7e.1 added a module-level `schemeListeners: Set<SchemeListener>` so that
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
MARKDOWN_PLUGINS_STANDALONE_WITH_BREAKS,
|
||||
MARKDOWN_PLUGINS_WITH_BREAKS,
|
||||
MARKDOWN_REHYPE_PLUGINS,
|
||||
isExternalHttpHref,
|
||||
} from '@/components/assistant-ui/markdown-text'
|
||||
|
||||
describe('MARKDOWN_PLUGINS integration', () => {
|
||||
@@ -97,3 +98,35 @@ describe('MARKDOWN_PLUGINS — currency prose vs KaTeX', () => {
|
||||
expect(html).toContain('class="katex"')
|
||||
})
|
||||
})
|
||||
|
||||
describe('isExternalHttpHref', () => {
|
||||
it('is true for http/https links', () => {
|
||||
expect(isExternalHttpHref('https://example.com')).toBe(true)
|
||||
expect(isExternalHttpHref('http://example.com/path?q=1#frag')).toBe(true)
|
||||
expect(isExternalHttpHref('HTTPS://Example.com')).toBe(true)
|
||||
})
|
||||
|
||||
it('is false for relative/SPA hrefs', () => {
|
||||
expect(isExternalHttpHref('/settings')).toBe(false)
|
||||
expect(isExternalHttpHref('./foo')).toBe(false)
|
||||
expect(isExternalHttpHref('#section')).toBe(false)
|
||||
expect(isExternalHttpHref('?q=1')).toBe(false)
|
||||
expect(isExternalHttpHref('/path:colon')).toBe(false)
|
||||
})
|
||||
|
||||
it('is false for non-http(s) schemes', () => {
|
||||
expect(isExternalHttpHref('mailto:a@b.com')).toBe(false)
|
||||
expect(isExternalHttpHref('vscode://file/foo')).toBe(false)
|
||||
expect(isExternalHttpHref('javascript:alert(1)')).toBe(false)
|
||||
})
|
||||
|
||||
it('is false for Windows drive paths (single-letter scheme, not http/https)', () => {
|
||||
expect(isExternalHttpHref('C:\\Users\\ian\\file.txt')).toBe(false)
|
||||
})
|
||||
|
||||
it('is false for empty/nullish input', () => {
|
||||
expect(isExternalHttpHref('')).toBe(false)
|
||||
expect(isExternalHttpHref(null)).toBe(false)
|
||||
expect(isExternalHttpHref(undefined)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -32,6 +32,7 @@ import { classifyNoSchemeHref } from '@/lib/markdown-href-policy'
|
||||
import { remarkSessionPathLinks } from '@/lib/remark-session-path-links'
|
||||
import { buildSessionReferencePath, parseSessionPathHref } from '@/lib/sessionReference'
|
||||
import { UriConfirmDialog } from '@/components/UriConfirmDialog'
|
||||
import { DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB, useOpenExternalLinksInNewTab } from '@/hooks/useOpenExternalLinksInNewTab'
|
||||
|
||||
import type { MarkdownTextPrimitiveProps } from '@assistant-ui/react-markdown'
|
||||
|
||||
@@ -217,6 +218,23 @@ function hasScheme(href: string): boolean {
|
||||
return boundaryIdx < 0 || colonIdx < boundaryIdx
|
||||
}
|
||||
|
||||
/**
|
||||
* True when href is a genuine external http(s) link — as opposed to a
|
||||
* relative/SPA href (no scheme, e.g. `/settings`) or another scheme
|
||||
* (`mailto:`, `vscode:`, a Windows drive path like `C:\Users\...`, etc).
|
||||
*
|
||||
* Used by <A> to decide whether the "open external links in a new tab"
|
||||
* setting (Settings > Display > Links) applies — forcing target="_blank" on
|
||||
* a relative href would open the whole app in a second tab instead of
|
||||
* navigating in-app, so this must stay scoped to real http(s) URLs.
|
||||
*/
|
||||
export function isExternalHttpHref(href: string | null | undefined): boolean {
|
||||
if (!href || !hasScheme(href)) return false
|
||||
const colonIdx = href.indexOf(':')
|
||||
const scheme = href.slice(0, colonIdx).toLowerCase()
|
||||
return scheme === 'http' || scheme === 'https'
|
||||
}
|
||||
|
||||
// ── URL sanitize transform (deny-only) ──────────────────────────────────────
|
||||
// Passed as urlTransform to MarkdownTextPrimitive. Only deny-listed schemes
|
||||
// are stripped; every other scheme (IANA + custom) passes through so the
|
||||
@@ -349,6 +367,8 @@ type UriConfirmContextValue = {
|
||||
openUri: (url: string, scheme: string) => void
|
||||
/** Shared isAllowed so all <a> tags in this tree re-render on the same state update. */
|
||||
isAllowed: (scheme: string) => boolean
|
||||
/** User preference (Settings > Display > Links): force target="_blank" on external http(s) links. */
|
||||
openExternalLinksInNewTab: boolean
|
||||
}
|
||||
|
||||
const UriConfirmContext = createContext<UriConfirmContextValue | null>(null)
|
||||
@@ -366,6 +386,7 @@ const UriConfirmContext = createContext<UriConfirmContextValue | null>(null)
|
||||
export function UriConfirmProvider({ children }: { children: ReactNode }) {
|
||||
const [dialog, setDialog] = useState<DialogState>(null)
|
||||
const { allow, isAllowed } = useAllowedSchemes()
|
||||
const { openExternalLinksInNewTab } = useOpenExternalLinksInNewTab()
|
||||
|
||||
const openUri = useCallback((url: string, scheme: string) => {
|
||||
setDialog({ url, scheme })
|
||||
@@ -387,7 +408,10 @@ export function UriConfirmProvider({ children }: { children: ReactNode }) {
|
||||
}
|
||||
}
|
||||
|
||||
const contextValue = useMemo(() => ({ openUri, isAllowed }), [openUri, isAllowed])
|
||||
const contextValue = useMemo(
|
||||
() => ({ openUri, isAllowed, openExternalLinksInNewTab }),
|
||||
[openUri, isAllowed, openExternalLinksInNewTab]
|
||||
)
|
||||
|
||||
return (
|
||||
<UriConfirmContext.Provider value={contextValue}>
|
||||
@@ -693,6 +717,13 @@ function A(props: ComponentPropsWithoutRef<'a'>) {
|
||||
const scheme = colonIdx > 0 && !isRelative ? href!.slice(0, colonIdx).toLowerCase() : ''
|
||||
const isCustomAllowed = classification === 'custom' && isAllowed(scheme)
|
||||
|
||||
// Settings > Display > Links toggle: force genuine external http(s) links
|
||||
// to open in a new browser tab instead of navigating same-tab.
|
||||
const openExternalLinksInNewTab = ctx?.openExternalLinksInNewTab ?? DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB
|
||||
const forceNewTab = openExternalLinksInNewTab && isExternalHttpHref(href)
|
||||
const target = forceNewTab ? '_blank' : props.target
|
||||
const effectiveRel = forceNewTab ? (rel ?? 'noopener noreferrer') : rel
|
||||
|
||||
const domHref =
|
||||
classification === 'iana' || isCustomAllowed
|
||||
? href
|
||||
@@ -732,7 +763,8 @@ function A(props: ComponentPropsWithoutRef<'a'>) {
|
||||
<a
|
||||
{...rest}
|
||||
href={domHref}
|
||||
rel={rel}
|
||||
target={target}
|
||||
rel={effectiveRel}
|
||||
onClick={handleClick}
|
||||
className={cn('aui-md-a font-medium text-[var(--app-link)] underline decoration-[color:var(--app-link-muted)] underline-offset-3', props.className)}
|
||||
/>
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
import {
|
||||
DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB,
|
||||
getInitialOpenExternalLinksInNewTab,
|
||||
useOpenExternalLinksInNewTab,
|
||||
} from './useOpenExternalLinksInNewTab'
|
||||
|
||||
const STORAGE_KEY = 'hapi-open-external-links-in-new-tab'
|
||||
|
||||
describe('useOpenExternalLinksInNewTab helpers', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear()
|
||||
})
|
||||
|
||||
it('defaults to disabled (today’s existing link behaviour)', () => {
|
||||
expect(getInitialOpenExternalLinksInNewTab()).toBe(DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB)
|
||||
expect(getInitialOpenExternalLinksInNewTab()).toBe(false)
|
||||
})
|
||||
|
||||
it('reads the stored value and falls back for other values', () => {
|
||||
window.localStorage.setItem(STORAGE_KEY, 'true')
|
||||
expect(getInitialOpenExternalLinksInNewTab()).toBe(true)
|
||||
|
||||
window.localStorage.setItem(STORAGE_KEY, 'invalid')
|
||||
expect(getInitialOpenExternalLinksInNewTab()).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('useOpenExternalLinksInNewTab', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear()
|
||||
})
|
||||
|
||||
it('persists changes and synchronizes hook instances in the same tab', () => {
|
||||
const first = renderHook(() => useOpenExternalLinksInNewTab())
|
||||
const second = renderHook(() => useOpenExternalLinksInNewTab())
|
||||
|
||||
act(() => first.result.current.setOpenExternalLinksInNewTab(true))
|
||||
|
||||
expect(first.result.current.openExternalLinksInNewTab).toBe(true)
|
||||
expect(second.result.current.openExternalLinksInNewTab).toBe(true)
|
||||
expect(window.localStorage.getItem(STORAGE_KEY)).toBe('true')
|
||||
|
||||
act(() => second.result.current.setOpenExternalLinksInNewTab(false))
|
||||
|
||||
expect(first.result.current.openExternalLinksInNewTab).toBe(false)
|
||||
expect(second.result.current.openExternalLinksInNewTab).toBe(false)
|
||||
expect(window.localStorage.getItem(STORAGE_KEY)).toBeNull()
|
||||
})
|
||||
|
||||
it('synchronizes changes from another browsing context', () => {
|
||||
const { result } = renderHook(() => useOpenExternalLinksInNewTab())
|
||||
|
||||
act(() => {
|
||||
window.dispatchEvent(new StorageEvent('storage', {
|
||||
key: STORAGE_KEY,
|
||||
newValue: 'true',
|
||||
}))
|
||||
})
|
||||
|
||||
expect(result.current.openExternalLinksInNewTab).toBe(true)
|
||||
})
|
||||
|
||||
it('ignores storage events for unrelated keys', () => {
|
||||
const { result } = renderHook(() => useOpenExternalLinksInNewTab())
|
||||
|
||||
act(() => {
|
||||
window.dispatchEvent(new StorageEvent('storage', {
|
||||
key: 'hapi-other-setting',
|
||||
newValue: 'false',
|
||||
}))
|
||||
})
|
||||
|
||||
expect(result.current.openExternalLinksInNewTab).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,99 @@
|
||||
import { useCallback, useEffect, useState } from 'react'
|
||||
|
||||
const OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY = 'hapi-open-external-links-in-new-tab'
|
||||
export const DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB = false
|
||||
|
||||
// Same-tab sync: `storage` events only fire in OTHER tabs/windows, but several
|
||||
// <UriConfirmProvider> instances can coexist in one window (e.g. one per
|
||||
// chat message), each with its own useState. Without this, toggling the
|
||||
// setting in Settings would only affect providers mounted after the change.
|
||||
const sameTabListeners = new Set<(value: boolean) => void>()
|
||||
|
||||
function isBrowser(): boolean {
|
||||
return typeof window !== 'undefined' && typeof document !== 'undefined'
|
||||
}
|
||||
|
||||
function safeGetItem(key: string): string | null {
|
||||
if (!isBrowser()) {
|
||||
return null
|
||||
}
|
||||
try {
|
||||
return localStorage.getItem(key)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function safeSetItem(key: string, value: string): void {
|
||||
if (!isBrowser()) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
localStorage.setItem(key, value)
|
||||
} catch {
|
||||
// Ignore storage errors
|
||||
}
|
||||
}
|
||||
|
||||
function safeRemoveItem(key: string): void {
|
||||
if (!isBrowser()) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
localStorage.removeItem(key)
|
||||
} catch {
|
||||
// Ignore storage errors
|
||||
}
|
||||
}
|
||||
|
||||
function parseOpenExternalLinksInNewTab(raw: string | null): boolean {
|
||||
if (raw === 'true') {
|
||||
return true
|
||||
}
|
||||
return DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB
|
||||
}
|
||||
|
||||
export function getInitialOpenExternalLinksInNewTab(): boolean {
|
||||
return parseOpenExternalLinksInNewTab(safeGetItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY))
|
||||
}
|
||||
|
||||
export function useOpenExternalLinksInNewTab(): {
|
||||
openExternalLinksInNewTab: boolean
|
||||
setOpenExternalLinksInNewTab: (value: boolean) => void
|
||||
} {
|
||||
const [openExternalLinksInNewTab, setOpenExternalLinksInNewTabState] = useState<boolean>(getInitialOpenExternalLinksInNewTab)
|
||||
|
||||
useEffect(() => {
|
||||
if (!isBrowser()) {
|
||||
return
|
||||
}
|
||||
|
||||
const onStorage = (event: StorageEvent) => {
|
||||
if (event.key !== OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY) {
|
||||
return
|
||||
}
|
||||
setOpenExternalLinksInNewTabState(parseOpenExternalLinksInNewTab(event.newValue))
|
||||
}
|
||||
|
||||
sameTabListeners.add(setOpenExternalLinksInNewTabState)
|
||||
window.addEventListener('storage', onStorage)
|
||||
return () => {
|
||||
sameTabListeners.delete(setOpenExternalLinksInNewTabState)
|
||||
window.removeEventListener('storage', onStorage)
|
||||
}
|
||||
}, [])
|
||||
|
||||
const setOpenExternalLinksInNewTab = useCallback((value: boolean) => {
|
||||
for (const listener of sameTabListeners) {
|
||||
listener(value)
|
||||
}
|
||||
|
||||
if (value === DEFAULT_OPEN_EXTERNAL_LINKS_IN_NEW_TAB) {
|
||||
safeRemoveItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY)
|
||||
} else {
|
||||
safeSetItem(OPEN_EXTERNAL_LINKS_IN_NEW_TAB_STORAGE_KEY, String(value))
|
||||
}
|
||||
}, [])
|
||||
|
||||
return { openExternalLinksInNewTab, setOpenExternalLinksInNewTab }
|
||||
}
|
||||
@@ -895,6 +895,9 @@ export default {
|
||||
'settings.display.sessionHeader.createdAt': 'Created time',
|
||||
'settings.display.sessionHeader.updatedAt': 'Updated time',
|
||||
'settings.display.sessionHeader.worktree': 'Worktree branch',
|
||||
'settings.display.links': 'Links',
|
||||
'settings.display.openExternalLinksInNewTab': 'Open external links in a new tab',
|
||||
'settings.display.openExternalLinksInNewTab.desc': 'Off (default) keeps today’s behaviour. On opens external links in a separate browser tab instead.',
|
||||
'session.header.createdAt': 'Created',
|
||||
'session.header.updatedAt': 'Updated',
|
||||
'settings.chat.title': 'Chat',
|
||||
|
||||
@@ -893,6 +893,9 @@ export default {
|
||||
'settings.display.sessionHeader.createdAt': '创建时间',
|
||||
'settings.display.sessionHeader.updatedAt': '更新时间',
|
||||
'settings.display.sessionHeader.worktree': '工作树分支',
|
||||
'settings.display.links': '链接',
|
||||
'settings.display.openExternalLinksInNewTab': '在新标签页中打开外部链接',
|
||||
'settings.display.openExternalLinksInNewTab.desc': '关闭(默认)保留当前行为。开启后外部链接会在单独的浏览器标签页中打开。',
|
||||
'session.header.createdAt': '创建',
|
||||
'session.header.updatedAt': '更新',
|
||||
'settings.chat.title': '聊天',
|
||||
|
||||
@@ -11,6 +11,7 @@ import { MAX_SESSION_PREVIEW_LIMIT, MIN_SESSION_PREVIEW_LIMIT, normalizeSessionP
|
||||
import { useThemeColors, type ThemeColorKeyId } from '@/hooks/useThemeColors'
|
||||
import { useSessionHeaderMetadata, type SessionHeaderMetadataKey } from '@/hooks/useSessionHeaderMetadata'
|
||||
import { useAppBadgePreference } from '@/hooks/useAppBadgePreference'
|
||||
import { useOpenExternalLinksInNewTab } from '@/hooks/useOpenExternalLinksInNewTab'
|
||||
import { SettingsChoiceGroup, SettingsFieldLabel, SettingsPageContent, SettingsRow, SettingsSection, SettingsSwitch } from '@/components/settings/SettingsPrimitives'
|
||||
|
||||
function MinusIcon() {
|
||||
@@ -140,6 +141,7 @@ export default function SettingsDisplayPage() {
|
||||
const { showActiveSessionsOnly, setShowActiveSessionsOnly } = useShowActiveSessionsOnly()
|
||||
const { pinInProgressSessions, setPinInProgressSessions } = usePinInProgressSessions()
|
||||
const { appBadgeEnabled, setAppBadgeEnabled } = useAppBadgePreference()
|
||||
const { openExternalLinksInNewTab, setOpenExternalLinksInNewTab } = useOpenExternalLinksInNewTab()
|
||||
const { preferences: sessionHeaderMetadata, setPreference: setSessionHeaderMetadata } = useSessionHeaderMetadata()
|
||||
const sessionHeaderOptions: ReadonlyArray<{ key: SessionHeaderMetadataKey; labelKey: string }> = [
|
||||
{ key: 'showLabels', labelKey: 'settings.display.sessionHeader.showLabels' },
|
||||
@@ -197,6 +199,15 @@ export default function SettingsDisplayPage() {
|
||||
/>
|
||||
))}
|
||||
</SettingsSection>
|
||||
|
||||
<SettingsSection title={t('settings.display.links')}>
|
||||
<SettingsSwitch
|
||||
label={t('settings.display.openExternalLinksInNewTab')}
|
||||
description={t('settings.display.openExternalLinksInNewTab.desc')}
|
||||
checked={openExternalLinksInNewTab}
|
||||
onChange={setOpenExternalLinksInNewTab}
|
||||
/>
|
||||
</SettingsSection>
|
||||
</SettingsPageContent>
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user