feat(editor): preserve Cmd+B for bold in markdown editor (#802)

* feat(editor): preserve Cmd+B for bold in markdown editor

Carve out bare Cmd/Ctrl+B from the main-process before-input-event
interceptor when the TipTap markdown editor is focused, so its bold
keymap can run instead of toggling the left sidebar. Focus state is
mirrored from renderer to main via a one-way IPC send, with default-deny
resets on crash/navigate/destroy and sender validation so only the main
window's webContents can mutate the flag.

* fix: add oxlint max-lines disable to createMainWindow.ts
This commit is contained in:
Jinjing 2026-04-18 10:08:25 -07:00 committed by GitHub
parent c2ed4fbd3b
commit 2688c0fee3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 452 additions and 3 deletions

View file

@ -131,7 +131,18 @@ fi
## Step 2: Create PR and Merge ## Step 2: Create PR and Merge
### 2a. Create PR ### 2a. Remove local design docs
Design/planning docs are local-only scratch files and must not be checked in.
Delete any untracked markdown files in `docs/` that match design/plan
naming patterns before pushing:
```bash
git ls-files --others --exclude-standard -- 'docs/*-design.md' 'docs/*-plan.md' 'docs/design-*.md' \
| xargs -I{} rm -f {}
```
### 2b. Create PR
**FIRST**: Push the branch to the remote so `gh pr create` doesn't fail with **FIRST**: Push the branch to the remote so `gh pr create` doesn't fail with
`aborted: you must first push the current branch to a remote`. The `aborted: you must first push the current branch to a remote`. The
@ -154,7 +165,7 @@ After `/create-pr` completes, extract PR info:
gh pr view --json number,url --jq '.number, .url' gh pr view --json number,url --jq '.number, .url'
``` ```
### 2b. Wait for CI Checks ### 2c. Wait for CI Checks
Wait **2.5 minutes** before the first poll: Wait **2.5 minutes** before the first poll:
@ -188,7 +199,7 @@ fi
- `PENDING` → Wait 90s (`sleep 90`, `timeout: 150000`), then poll again - `PENDING` → Wait 90s (`sleep 90`, `timeout: 150000`), then poll again
- `NO_CHECKS` → Proceed to merge - `NO_CHECKS` → Proceed to merge
### 2c. Merge ### 2d. Merge
```bash ```bash
IS_WORKTREE=false IS_WORKTREE=false

5
.gitignore vendored
View file

@ -49,6 +49,11 @@ tmp/
design-docs/ design-docs/
.context/ .context/
# Local-only design/planning docs (not checked in)
docs/*-design.md
docs/*-plan.md
docs/design-*.md
# Stably CLI (only docs/ are tracked) # Stably CLI (only docs/ are tracked)
.stably/* .stably/*
!.stably/docs/ !.stably/docs/

View file

@ -500,6 +500,342 @@ describe('createMainWindow', () => {
expect(browserWindowInstance.setWindowButtonPosition).not.toHaveBeenCalled() expect(browserWindowInstance.setWindowButtonPosition).not.toHaveBeenCalled()
}) })
it('intercepts Cmd+B for sidebar when the markdown editor is not focused', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const preventDefault = vi.fn()
const isDarwin = process.platform === 'darwin'
windowHandlers['before-input-event'](
{ preventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
)
expect(preventDefault).toHaveBeenCalledTimes(1)
expect(webContents.send).toHaveBeenCalledWith('ui:toggleLeftSidebar')
})
it('skips Cmd+B interception when the markdown editor is focused', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const setFocusedListener = vi
.mocked(ipcMain.on)
.mock.calls.find(([channel]) => channel === 'ui:setMarkdownEditorFocused')?.[1]
expect(setFocusedListener).toBeTypeOf('function')
setFocusedListener?.({ sender: webContents } as never, true)
const preventDefault = vi.fn()
const isDarwin = process.platform === 'darwin'
windowHandlers['before-input-event'](
{ preventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
)
expect(preventDefault).not.toHaveBeenCalled()
expect(webContents.send).not.toHaveBeenCalledWith('ui:toggleLeftSidebar')
})
it('still intercepts Cmd+Shift+B and Cmd+Alt+B when the markdown editor is focused', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const setFocusedListener = vi
.mocked(ipcMain.on)
.mock.calls.find(([channel]) => channel === 'ui:setMarkdownEditorFocused')?.[1]
setFocusedListener?.({ sender: webContents } as never, true)
const isDarwin = process.platform === 'darwin'
// Cmd+Shift+B is not in the policy allowlist, so no action resolves and no
// preventDefault fires — but the carve-out must not be what lets it through.
const shiftPreventDefault = vi.fn()
windowHandlers['before-input-event'](
{ preventDefault: shiftPreventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'B',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: true
} as never
)
expect(shiftPreventDefault).not.toHaveBeenCalled()
// Cmd+Alt+B is not a modifier chord in the policy (alt excluded), so the
// policy returns null and no preventDefault fires. Assert the carve-out
// is not what's short-circuiting this — it requires !alt.
const altPreventDefault = vi.fn()
windowHandlers['before-input-event'](
{ preventDefault: altPreventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: true,
shift: false
} as never
)
expect(altPreventDefault).not.toHaveBeenCalled()
})
it('coerces non-boolean setMarkdownEditorFocused payloads to false', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const setFocusedListener = vi
.mocked(ipcMain.on)
.mock.calls.find(([channel]) => channel === 'ui:setMarkdownEditorFocused')?.[1]
// Seed to true with a legitimate payload, then send a non-boolean and
// assert the flag returns to false by checking Cmd+B resumes interception.
setFocusedListener?.({ sender: webContents } as never, true)
setFocusedListener?.({ sender: webContents } as never, { malicious: true } as never)
const preventDefault = vi.fn()
const isDarwin = process.platform === 'darwin'
windowHandlers['before-input-event'](
{ preventDefault } as never,
{
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
)
expect(preventDefault).toHaveBeenCalledTimes(1)
expect(webContents.send).toHaveBeenCalledWith('ui:toggleLeftSidebar')
})
it('resets the markdown editor focus flag on renderer crash, navigation, and destroy', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = {
on: vi.fn((event, handler) => {
windowHandlers[event] = handler
}),
setZoomLevel: vi.fn(),
setBackgroundThrottling: vi.fn(),
invalidate: vi.fn(),
setWindowOpenHandler: vi.fn(),
send: vi.fn(),
isDevToolsOpened: vi.fn(),
openDevTools: vi.fn(),
closeDevTools: vi.fn()
}
const browserWindowInstance = {
webContents,
on: vi.fn(),
isDestroyed: vi.fn(() => false),
isMaximized: vi.fn(() => true),
isFullScreen: vi.fn(() => false),
getSize: vi.fn(() => [1200, 800]),
setSize: vi.fn(),
maximize: vi.fn(),
show: vi.fn(),
loadFile: vi.fn(),
loadURL: vi.fn()
}
browserWindowMock.mockImplementation(function () {
return browserWindowInstance
})
createMainWindow(null)
const setFocusedListener = vi
.mocked(ipcMain.on)
.mock.calls.find(([channel]) => channel === 'ui:setMarkdownEditorFocused')?.[1]
const isDarwin = process.platform === 'darwin'
const cmdBInput = {
type: 'keyDown',
code: 'KeyB',
key: 'b',
meta: isDarwin,
control: !isDarwin,
alt: false,
shift: false
} as never
const assertInterceptsAfterReset = (): void => {
webContents.send.mockClear()
const preventDefault = vi.fn()
windowHandlers['before-input-event']({ preventDefault } as never, cmdBInput)
expect(preventDefault).toHaveBeenCalledTimes(1)
expect(webContents.send).toHaveBeenCalledWith('ui:toggleLeftSidebar')
}
// render-process-gone
setFocusedListener?.({ sender: webContents } as never, true)
windowHandlers['render-process-gone']?.()
assertInterceptsAfterReset()
// did-start-navigation (main frame)
setFocusedListener?.({ sender: webContents } as never, true)
windowHandlers['did-start-navigation']?.({} as never, 'https://example.com/', false, true)
assertInterceptsAfterReset()
// did-start-navigation (sub-frame) should NOT reset the flag
setFocusedListener?.({ sender: webContents } as never, true)
windowHandlers['did-start-navigation']?.({} as never, 'https://example.com/', false, false)
webContents.send.mockClear()
const subframePreventDefault = vi.fn()
windowHandlers['before-input-event'](
{ preventDefault: subframePreventDefault } as never,
cmdBInput
)
expect(subframePreventDefault).not.toHaveBeenCalled()
expect(webContents.send).not.toHaveBeenCalledWith('ui:toggleLeftSidebar')
// destroyed
setFocusedListener?.({ sender: webContents } as never, true)
windowHandlers['destroyed']?.()
assertInterceptsAfterReset()
})
it('ignores duplicate ready-to-show events after startup maximize has already run', () => { it('ignores duplicate ready-to-show events after startup maximize has already run', () => {
const windowHandlers: Record<string, (...args: any[]) => void> = {} const windowHandlers: Record<string, (...args: any[]) => void> = {}
const webContents = { const webContents = {

View file

@ -1,3 +1,4 @@
/* oxlint-disable max-lines */
import { BrowserWindow, ipcMain, nativeTheme, screen, shell } from 'electron' import { BrowserWindow, ipcMain, nativeTheme, screen, shell } from 'electron'
import { join } from 'path' import { join } from 'path'
import { is } from '@electron-toolkit/utils' import { is } from '@electron-toolkit/utils'
@ -262,6 +263,44 @@ export function createMainWindow(
event.preventDefault() event.preventDefault()
}) })
// Why: mirrors the renderer's markdown-editor focus state so the main-process
// before-input-event handler can skip Cmd/Ctrl+B interception while TipTap
// owns focus. See docs/markdown-cmd-b-bold-design.md. We only carve out
// Cmd+B — terminal and browser-guest focus still get sidebar-toggle, which
// preserves the ^B-to-PTY leak protection rationale in
// shared/window-shortcut-policy.ts:74-77.
let markdownEditorFocused = false
const markdownFocusChannel = 'ui:setMarkdownEditorFocused'
// Why: coerce to strict boolean and verify the sender. A renderer bug or
// compromised IPC payload must not set the flag to a truthy non-bool (e.g.
// an object) and silently disable the sidebar toggle — default-deny on any
// non-bool. Additionally, only this main window's top-level webContents may
// mutate the flag, so a guest/webview or unrelated sender can't disable the
// Cmd+B sidebar carve-out.
const onMarkdownEditorFocused = (event: Electron.IpcMainEvent, focused: unknown): void => {
if (event.sender !== mainWindow.webContents) {
return
}
markdownEditorFocused = focused === true
}
ipcMain.on(markdownFocusChannel, onMarkdownEditorFocused)
// Why: renderer can't mirror focus state across a crash/reload/close.
// Default-deny the carve-out so Cmd+B falls back to sidebar-toggle, which is
// the safe behavior when focus context is unknown. Preserves the
// ^B-to-PTY leak invariant from shared/window-shortcut-policy.ts:74-77.
const resetMarkdownEditorFocus = (): void => {
markdownEditorFocused = false
}
mainWindow.webContents.on('render-process-gone', resetMarkdownEditorFocus)
mainWindow.webContents.on('destroyed', resetMarkdownEditorFocus)
mainWindow.webContents.on('did-start-navigation', (_e, _url, _isInPlace, isMainFrame) => {
if (isMainFrame) {
resetMarkdownEditorFocus()
}
})
mainWindow.webContents.on('before-input-event', (event, input) => { mainWindow.webContents.on('before-input-event', (event, input) => {
if (input.type !== 'keyDown') { if (input.type !== 'keyDown') {
return return
@ -277,6 +316,24 @@ export function createMainWindow(
return return
} }
// Why: TipTap owns bare Cmd/Ctrl+B for bold while the markdown editor is
// focused — skip interception so its keymap runs. Scoped to the bare chord
// (no Shift/Alt): any extra modifier signals different intent and must
// still resolve through the policy allowlist. Other focus contexts
// (terminal, browser guest) still get sidebar-toggle because ^B would
// otherwise reach xterm.js / guest webContents.
// See docs/markdown-cmd-b-bold-design.md.
const modForBold = process.platform === 'darwin' ? input.meta : input.control
if (
markdownEditorFocused &&
input.code === 'KeyB' &&
!input.alt &&
!input.shift &&
modForBold
) {
return
}
// Why: keep the main-process interception surface as an explicit allowlist. // Why: keep the main-process interception surface as an explicit allowlist.
// Anything outside this helper must continue to the renderer/PTTY so // Anything outside this helper must continue to the renderer/PTTY so
// readline control chords are not silently stolen above the terminal. // readline control chords are not silently stolen above the terminal.
@ -369,8 +426,13 @@ export function createMainWindow(
ipcMain.on(confirmCloseChannel, onConfirmClose) ipcMain.on(confirmCloseChannel, onConfirmClose)
mainWindow.on('closed', () => { mainWindow.on('closed', () => {
// Why: default-deny the Cmd+B carve-out after the window is gone so a
// stale-true flag can't leak past subsequent state transitions. Paired
// with the webContents lifecycle resets above.
markdownEditorFocused = false
ipcMain.removeListener(trafficLightChannel, onSyncTrafficLights) ipcMain.removeListener(trafficLightChannel, onSyncTrafficLights)
ipcMain.removeListener(confirmCloseChannel, onConfirmClose) ipcMain.removeListener(confirmCloseChannel, onConfirmClose)
ipcMain.removeListener(markdownFocusChannel, onMarkdownEditorFocused)
}) })
if (is.dev && process.env.ELECTRON_RENDERER_URL) { if (is.dev && process.env.ELECTRON_RENDERER_URL) {

View file

@ -602,6 +602,7 @@ export type PreloadApi = {
getZoomLevel: () => number getZoomLevel: () => number
setZoomLevel: (level: number) => void setZoomLevel: (level: number) => void
syncTrafficLights: (zoomFactor: number) => void syncTrafficLights: (zoomFactor: number) => void
setMarkdownEditorFocused: (focused: boolean) => void
onFullscreenChanged: (callback: (isFullScreen: boolean) => void) => () => void onFullscreenChanged: (callback: (isFullScreen: boolean) => void) => () => void
onWindowCloseRequested: (callback: (data: { isQuitting: boolean }) => void) => () => void onWindowCloseRequested: (callback: (data: { isQuitting: boolean }) => void) => () => void
confirmWindowClose: () => void confirmWindowClose: () => void

View file

@ -1129,6 +1129,13 @@ const api = {
setZoomLevel: (level: number): void => webFrame.setZoomLevel(level), setZoomLevel: (level: number): void => webFrame.setZoomLevel(level),
syncTrafficLights: (zoomFactor: number): void => syncTrafficLights: (zoomFactor: number): void =>
ipcRenderer.send('ui:sync-traffic-lights', zoomFactor), ipcRenderer.send('ui:sync-traffic-lights', zoomFactor),
// Why: one-way send (not invoke) so the main-process before-input-event
// handler can read the mirrored flag synchronously without a round-trip.
// The carve-out in createMainWindow.ts uses this to skip Cmd+B interception
// while the markdown editor owns focus, letting TipTap apply bold instead.
setMarkdownEditorFocused: (focused: boolean): void => {
ipcRenderer.send('ui:setMarkdownEditorFocused', focused)
},
onFullscreenChanged: (callback: (isFullScreen: boolean) => void): (() => void) => { onFullscreenChanged: (callback: (isFullScreen: boolean) => void): (() => void) => {
const listener = (_event: Electron.IpcRendererEvent, isFullScreen: boolean) => const listener = (_event: Electron.IpcRendererEvent, isFullScreen: boolean) =>
callback(isFullScreen) callback(isFullScreen)

View file

@ -494,6 +494,12 @@ function App(): React.JSX.Element {
// browser guest has focus. The renderer keeps matching handlers for // browser guest has focus. The renderer keeps matching handlers for
// local-focus cases and to preserve the same guards in one place. // local-focus cases and to preserve the same guards in one place.
// Why: keep this guard. TipTap's Cmd+B bold binding depends on the
// window-level handler *not* toggling the sidebar when focus lives in an
// editable surface. The main-process before-input-event already carves out
// Cmd+B for the markdown editor (see createMainWindow.ts +
// docs/markdown-cmd-b-bold-design.md), but this renderer-side fallback
// still covers the blur→press IPC race and any non-carved editable surface.
if (isEditableTarget(e.target)) { if (isEditableTarget(e.target)) {
return return
} }

View file

@ -162,6 +162,16 @@ export default function RichMarkdownEditor({
return false return false
} }
}, },
onFocus: () => {
// Why: mirror TipTap focus into the main process so the before-input-event
// Cmd+B carve-out in createMainWindow.ts lets the bold keymap run instead
// of intercepting the chord for sidebar toggle.
// See docs/markdown-cmd-b-bold-design.md.
window.api.ui.setMarkdownEditorFocused(true)
},
onBlur: () => {
window.api.ui.setMarkdownEditorFocused(false)
},
onCreate: ({ editor: nextEditor }) => { onCreate: ({ editor: nextEditor }) => {
// Why: markdown soft line breaks produce paragraphs with embedded `\n` chars. // Why: markdown soft line breaks produce paragraphs with embedded `\n` chars.
// Normalizing them into separate paragraph nodes on load ensures Cmd+X (and // Normalizing them into separate paragraph nodes on load ensures Cmd+X (and
@ -234,6 +244,17 @@ export default function RichMarkdownEditor({
editorRef.current = editor ?? null editorRef.current = editor ?? null
}, [editor]) }, [editor])
// Why: TipTap's onBlur may not fire on unmount paths (tab close, HMR,
// component teardown while focused), leaving the main-process flag stale at
// `true` and silently disabling Cmd+B sidebar-toggle until the next editor
// focus/blur cycle. Force a `false` on unmount as a belt-and-braces reset.
// See docs/markdown-cmd-b-bold-design.md "Stale-flag recovery".
useEffect(() => {
return () => {
window.api.ui.setMarkdownEditorFocused(false)
}
}, [])
// Why: use useLayoutEffect (synchronous cleanup) so the pending serialization // Why: use useLayoutEffect (synchronous cleanup) so the pending serialization
// flush runs before useEditor's cleanup destroys the editor instance on tab // flush runs before useEditor's cleanup destroys the editor instance on tab
// switch or mode change. React runs layout-effect cleanups before effect // switch or mode change. React runs layout-effect cleanups before effect