Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 170 additions & 0 deletions apps/desktop/src/main/__tests__/shortcut-label-platform.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/**
* What the shell's two shortcut surfaces SAY, on all three platforms.
*
* The palette hints and the shortcuts sheet used to be written into both locale
* catalogs as macOS glyphs, so a Windows user was told to press ⌥⌘S for a side
* chat that answers to Ctrl+Alt+S (#3876). These read the real catalogs through
* the real formatter — the same call the panel and the palette make — so a glyph
* reintroduced into either catalog fails here rather than shipping.
*/

import assert from 'node:assert/strict';
import test from 'node:test';
import { formatShortcut } from '@maka/ui';
import type { UiLocale } from '@maka/core/ui-locale';
import { buildCommandList } from '../../renderer/command-palette-commands.js';
import { getShellCopy } from '../../renderer/locales/shell-copy.js';

const PLATFORMS = ['darwin', 'win32', 'linux'] as const;
const LOCALES: readonly UiLocale[] = ['en', 'zh'];
const APPLE_GLYPHS = /[\u2318\u2325\u21E7\u2303]/u;

function hints(locale: UiLocale, platform: string): Map<string, string | undefined> {
const commands = buildCommandList({
locale,
platform,
activeSessionId: 'session-1',
themePref: 'auto',
connections: [],
defaultSlug: null,
onNewChat: () => {},
onOpenSideChat: () => {},
onOpenSettings: () => {},
onOpenSettingsSection: () => {},
onOpenShortcuts: () => {},
onSetTheme: () => {},
// The diagnostics rows only register when the shell wires their action, and
// two of them are what this file is about.
onCopyDiagnostics: () => {},
onOpenWorkspace: () => {},
});
return new Map(commands.map((command) => [command.id, command.hint]));
}

test('palette hints lead with the shortcut this platform answers to', () => {
const mac = hints('en', 'darwin');
assert.equal(mac.get('action:open-settings'), '⌘,');
assert.equal(mac.get('action:side-chat'), '⌥⌘S');

for (const platform of ['win32', 'linux']) {
const pc = hints('en', platform);
assert.equal(pc.get('action:open-settings'), 'Ctrl+,');
assert.equal(pc.get('action:side-chat'), 'Ctrl+Alt+S');
}
});

test('a hint that also carries prose keeps the prose, in its own locale', () => {
// `⇧⌘D · Redacted logs · clipboard only` was one hard-coded string. The keys
// are now derived and the sentence after them is still the catalog's.
assert.equal(
hints('en', 'darwin').get('diag:copy-diagnostics'),
'⇧⌘D · Redacted logs · clipboard only',
);
assert.equal(
hints('en', 'win32').get('diag:copy-diagnostics'),
'Ctrl+Shift+D · Redacted logs · clipboard only',
);
assert.equal(
hints('zh', 'win32').get('diag:copy-diagnostics'),
'Ctrl+Shift+D · 脱敏日志 · 仅写入剪贴板',
);
});

test('a command without a shortcut keeps the hint the catalog wrote', () => {
// Positive control: the derivation must not reach commands it was never
// about, or every hint in the palette would grow a shortcut it does not have.
assert.equal(hints('en', 'win32').get('action:new-chat'), 'Start a new task');
assert.equal(hints('en', 'win32').get('action:keyboard-help'), '?');
assert.equal(hints('zh', 'darwin').get('diag:open-workspace'), 'Finder');
});

test('no palette hint spells a modifier with an Apple glyph off macOS', () => {
for (const locale of LOCALES) {
for (const platform of ['win32', 'linux']) {
for (const [id, hint] of hints(locale, platform)) {
assert.ok(
!hint || !APPLE_GLYPHS.test(hint),
`${locale}/${platform} hint for ${id} still reads ${hint}`,
);
}
}
}
});

test('the shortcuts sheet reads Ctrl off macOS and ⌘ on it', () => {
for (const locale of LOCALES) {
const rows = getShellCopy(locale).keyboardHelp.sections.flatMap((section) => section.rows);
const general = new Map(
rows.map((row) => [row.keys.join('+'), row.keys] as const),
);
const newTask = general.get('mod+n');
const palette = general.get('mod+k');
const diagnostics = general.get('mod+shift+d');
const lineBreak = general.get('alt+enter');
assert.ok(newTask && palette && diagnostics && lineBreak, `${locale} sheet lost a row`);

assert.equal(formatShortcut(newTask, 'darwin'), '⌘N');
assert.equal(formatShortcut(palette, 'darwin'), '⌘K');
assert.equal(formatShortcut(diagnostics, 'darwin'), '⇧⌘D');
assert.equal(formatShortcut(lineBreak, 'darwin'), '⌥↵');

for (const platform of ['win32', 'linux']) {
assert.equal(formatShortcut(newTask, platform), 'Ctrl+N');
assert.equal(formatShortcut(palette, platform), 'Ctrl+K');
assert.equal(formatShortcut(diagnostics, platform), 'Ctrl+Shift+D');
assert.equal(formatShortcut(lineBreak, platform), 'Alt+↵');
}
}
});

test('the sheet is authored in tokens, so no row can be macOS-only', () => {
// The bug was a glyph in the copy, not in the renderer. A row that reaches
// the panel already spelled has no platform left to answer to.
for (const locale of LOCALES) {
for (const section of getShellCopy(locale).keyboardHelp.sections) {
for (const row of section.rows) {
for (const key of row.keys) {
assert.ok(
!APPLE_GLYPHS.test(key),
`${locale} sheet row "${row.description}" hard-codes ${key}`,
);
}
}
}
}
});

test('every sheet row renders something on every platform', () => {
for (const locale of LOCALES) {
for (const platform of PLATFORMS) {
for (const section of getShellCopy(locale).keyboardHelp.sections) {
for (const row of section.rows) {
const rendered = formatShortcut(row.keys, platform);
assert.ok(
rendered.trim().length > 0,
`${locale}/${platform} row "${row.description}" rendered nothing`,
);
}
}
}
}
});
3 changes: 3 additions & 0 deletions apps/desktop/src/renderer/app-shell-command-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type RefBox<T> = { current: T };

export interface AppShellCommandListOptions {
uiLocale: UiLocale;
/** Host OS, so shortcut hints read `Ctrl+,` off macOS and `⌘,` on it. */
hostPlatform: string | undefined;
activeId: string | undefined;
activePermissionMode: PermissionMode | undefined;
canSetPermissionMode: boolean;
Expand Down Expand Up @@ -131,6 +133,7 @@ export function buildAppShellCommandList(

return buildCommandList({
locale: options.uiLocale,
platform: options.hostPlatform,
activeSessionId: options.activeId,
themePref: options.themePref,
connections: options.connections,
Expand Down
36 changes: 27 additions & 9 deletions apps/desktop/src/renderer/app-shell-effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import { useEffect, useEffectEvent, useLayoutEffect } from 'react';
import { useEffect, useEffectEvent, useLayoutEffect, useState } from 'react';
import { useHotkeys } from '@astryxdesign/core/hooks';
import type { ConnectionEvent } from '@maka/core/connections';
import type { SessionChangedEvent, SessionSummary, StoredMessage } from '@maka/core/session';
Expand Down Expand Up @@ -87,19 +87,33 @@ export function useAppShellNavRefSync(options: { navSelection: NavSelection; nav
}, [options.navSelection]);
}

export function useAppShellHostEffects() {
// Tag the document with the host OS so glass-material CSS rules
// (sidebar vibrancy passthrough)
// can light up only on macOS, where `BrowserWindow({ vibrancy: 'sidebar' })`
// paints the native blur material behind the renderer. Other platforms
// keep their opaque chrome since vibrancy is a no-op there.
/**
* The host OS, once the main process names it.
*
* Two readers, one round trip. It is tagged onto the document so
* glass-material CSS rules (sidebar vibrancy passthrough) can light up only on
* macOS, where `BrowserWindow({ vibrancy: 'sidebar' })` paints the native blur
* material behind the renderer — other platforms keep their opaque chrome
* since vibrancy is a no-op there. It is returned so `HostPlatformProvider`
* can hand it to every surface that shows a keyboard shortcut, which has to
* spell it `Ctrl` here and `⌘` there (#3876).
*
* Undefined until the answer arrives, and undefined forever if it never does.
* No label waits on it: the shortcut formatter reads `navigator` in the
* meantime, so the only thing an unanswered `app.info()` costs is the vibrancy
* rules, exactly as before.
*/
export function useResolvedHostPlatform(): string | undefined {
const [platform, setPlatform] = useState<string | undefined>(undefined);

useEffect(() => {
let cancelled = false;
void window.maka.app
.info()
.then((info) => {
if (cancelled) return;
document.documentElement.setAttribute('data-os', info.platform);
if (cancelled) return;
document.documentElement.setAttribute('data-os', info.platform);
setPlatform(info.platform);
})
.catch(() => {
/* swallow — leaves data-os unset, CSS falls back to opaque chrome */
Expand All @@ -109,6 +123,10 @@ export function useAppShellHostEffects() {
};
}, []);

return platform;
}

export function useAppShellHostEffects() {
// Modal-open titlebar dimming/hiding is driven by observing the top layer
// (`dialog:modal`) rather than the shell's own modal state, so dialogs
// mounted deep in module pages — the scheduled-task form above all — are
Expand Down
33 changes: 22 additions & 11 deletions apps/desktop/src/renderer/app-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
type MakaUriDest,
MakaUriContext,
AstryxLocaleProvider,
HostPlatformProvider,
LocaleProvider,
ToastProvider,
type ToastDiagnosticTarget,
Expand All @@ -67,6 +68,7 @@ import {
enqueueInteraction,
getConversationCopy,
reconcileInteractions,
useHostPlatform,
} from '@maka/ui';
import type { ConnectionEvent } from '@maka/core/connections';
import { GitBranch, MessageCircleQuestion, Minimize2, Network } from '@maka/ui/icons';
Expand Down Expand Up @@ -193,6 +195,7 @@ import {
useAppShellHostEffects,
useAppShellPersistenceEffects,
useAppShellNavRefSync,
useResolvedHostPlatform,
useSessionEventHealthPolling,
useShellRunUpdates,
} from './app-shell-effects';
Expand Down Expand Up @@ -280,6 +283,7 @@ export function AppShell({ initialOnboardingSnapshot = null }: AppShellProps = {
const [uiLocaleOverride, setUiLocaleOverride] = useState<UiLocale | null>(null);
const systemUiLocale = useSystemUiLocale();
const uiLocale = resolveUiLocale(uiLocalePreference, systemUiLocale, uiLocaleOverride);
const hostPlatform = useResolvedHostPlatform();
const errorToastAction = useMemo<ToastErrorAction>(
() => ({
label: getShellCopy(uiLocale).errorBoundary.copyReport,
Expand All @@ -303,17 +307,22 @@ export function AppShell({ initialOnboardingSnapshot = null }: AppShellProps = {
`useUiLocale()` throws before anything renders. Still above every
Astryx subtree. */}
<AstryxLocaleProvider>
<ToastProvider errorAction={errorToastAction}>
<ErrorBoundary locale={uiLocale}>
<AppShellContent
initialOnboardingSnapshot={initialOnboardingSnapshot}
uiLocale={uiLocale}
uiLocaleOverride={uiLocaleOverride}
setUiLocaleOverride={setUiLocaleOverride}
setUiLocalePreference={setUiLocalePreference}
/>
</ErrorBoundary>
</ToastProvider>
{/* Above the shell, not inside it: the rail's new-task hint and the
shortcuts sheet both spell `mod` per platform, and they sit in
different subtrees. */}
<HostPlatformProvider platform={hostPlatform}>
<ToastProvider errorAction={errorToastAction}>
<ErrorBoundary locale={uiLocale}>
<AppShellContent
initialOnboardingSnapshot={initialOnboardingSnapshot}
uiLocale={uiLocale}
uiLocaleOverride={uiLocaleOverride}
setUiLocaleOverride={setUiLocaleOverride}
setUiLocalePreference={setUiLocalePreference}
/>
</ErrorBoundary>
</ToastProvider>
</HostPlatformProvider>
</AstryxLocaleProvider>
</LocaleProvider>
);
Expand All @@ -333,6 +342,7 @@ function AppShellContent({
setUiLocalePreference: Dispatch<SetStateAction<UiLocalePreference>>;
}) {
const toastApi = useToast();
const hostPlatform = useHostPlatform();
const [appUpdateStatus, setAppUpdateStatus] = useState<AppUpdateStatus | null>(null);
const updateInstallInFlightRef = useRef(false);
const notifiedInstallErrorRef = useRef<string | null>(null);
Expand Down Expand Up @@ -2598,6 +2608,7 @@ function AppShellContent({
!activeMessageLoadError;
const commandOptions: AppShellCommandListOptions = {
uiLocale,
hostPlatform,
activeId,
activePermissionMode,
canSetPermissionMode: activeBoundarySurface.localInteractionAvailable,
Expand Down
20 changes: 17 additions & 3 deletions apps/desktop/src/renderer/command-palette-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ import { isRetiredProvider } from '@maka/core/provider-registry';
import type { PermissionMode } from '@maka/core/permission';
import type { SessionSummary } from '@maka/core/session';
import type { UiLocale } from '@maka/core/ui-locale';
import type { NavSelection } from '@maka/ui';
import { getShellCopy } from './locales/shell-copy.js';
import { formatShortcut, type NavSelection } from '@maka/ui';
import { getShellCopy, STATIC_COMMAND_SHORTCUTS } from './locales/shell-copy.js';
import { SETTINGS_NAV } from './settings/settings-nav.js';
import type { Command } from './command-palette-types.js';

Expand All @@ -64,6 +64,11 @@ import type { Command } from './command-palette-types.js';
*/
export function buildCommandList(args: {
locale: UiLocale;
/**
* Host OS, so a hint spells its shortcut the way this platform does. Omitted
* outside the desktop shell, where the formatter falls back to `navigator`.
*/
platform?: string;
activeSessionId: string | undefined;
themePref: ThemePreference;
connections: LlmConnection[];
Expand Down Expand Up @@ -147,7 +152,16 @@ export function buildCommandList(args: {
onStartScheduledTask?(): void;
}): Command[] {
const copy = getShellCopy(args.locale).commandPalette;
const staticCopy = (id: keyof typeof copy.commands) => copy.commands[id];
// A shortcut leads the hint, and the localized detail follows it behind the
// separator this file already uses everywhere else ("诊断 · 不打开设置").
// Commands without a shortcut keep their hint exactly as written.
const staticCopy = (id: keyof typeof copy.commands) => {
const entry = copy.commands[id];
const keys = STATIC_COMMAND_SHORTCUTS[id];
if (!keys) return entry;
const shortcut = formatShortcut(keys, args.platform);
return { ...entry, hint: entry.hint ? `${shortcut} · ${entry.hint}` : shortcut };
};
const cmds: Command[] = [
{
id: 'action:new-chat',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import { Card } from '@astryxdesign/core/Card';
import { ContextMenu } from '@astryxdesign/core/ContextMenu';
import { Heading } from '@astryxdesign/core/Heading';
import { Icon } from '@astryxdesign/core/Icon';
import { Kbd } from '@astryxdesign/core/Kbd';
import { List, ListItem } from '@astryxdesign/core/List';
import { Section } from '@astryxdesign/core/Section';
import { Spinner } from '@astryxdesign/core/Spinner';
Expand All @@ -88,6 +87,7 @@ import {
terminalRefFromWorkbarTab,
} from '../model/workbar-tabs';
import { useSessionTasks } from '../tools/tasks/use-session-tasks';
import { ShortcutKeys } from '../../../shortcut-keys';
import { WorkbarToggle } from './workbar-toggle';
import { WorkBoardPanel } from '../../../work-board-panel.js';
import { getDesktopConversationCopy } from '../../../locales/conversation-copy.js';
Expand Down Expand Up @@ -660,7 +660,7 @@ function WorkbarLauncher(props: {
description={action.description}
endContent={
action.shortcut ? (
<Kbd keys={action.shortcut} />
<ShortcutKeys keys={action.shortcut} />
) : undefined
}
isDisabled={action.disabled}
Expand Down
Loading