From c1a73fa5b0b8af2c9d05d1ae98a69e6aa1d0153f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 23 Jun 2026 15:01:30 +0000 Subject: [PATCH 01/11] Initial plan From 5c7df6f66d21f36197741423d2f10bfef1164943 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 06:07:50 +0000 Subject: [PATCH 02/11] fix(command-center): keep open during multicursor command execution (#3995) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com> --- .../__tests__/multicursorAlertMiddleware.ts | 34 +++++++++++++++++++ .../multicursorAlertMiddleware.ts | 7 +++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts b/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts index 64a7ccc5a24..8e3fe2e8ee9 100644 --- a/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts +++ b/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts @@ -2,6 +2,7 @@ import { importTextActionCreator as importText } from '../../actions/importText' import { toggleDropdownActionCreator as toggleDropdown } from '../../actions/toggleDropdown' import { undoActionCreator as undo } from '../../actions/undo' import { executeCommandWithMulticursor } from '../../commands' +import boldCommand from '../../commands/bold' import deleteCommand from '../../commands/delete' import { initialize } from '../../initialize' import store from '../../stores/app' @@ -35,6 +36,39 @@ it('shows the Command Center on mobile when a multicursor is active', async () = expect(store.getState().showCommandCenter).toBe(true) }) +it('keeps the Command Center open while a multicursor formatting command executes', async () => { + await initialize() + + store.dispatch([ + importText({ + text: ` + - A + - B + - C`, + }), + setCursor(['C']), + // open the Command Center by activating a multicursor on the cursor thought (swipe up) + addMulticursor(['C']), + ]) + expect(store.getState().showCommandCenter).toBe(true) + + // Track whether the Command Center is ever closed mid-command. + // executeCommandWithMulticursor transiently clears and restores the multicursors, which previously flickered + // showCommandCenter true → false → true and re-animated the sheet on iOS. + let closedDuringExecution = false + const unsubscribe = store.subscribe(() => { + if (!store.getState().showCommandCenter) closedDuringExecution = true + }) + + // apply a formatting command (Bold) across the multicursor + executeCommandWithMulticursor(boldCommand, { store }) + + unsubscribe() + + expect(closedDuringExecution).toBe(false) + expect(store.getState().showCommandCenter).toBe(true) +}) + it('does not show the Command Center when undoing a multicursor delete while the Undo Slider is active', async () => { await initialize() diff --git a/src/redux-middleware/multicursorAlertMiddleware.ts b/src/redux-middleware/multicursorAlertMiddleware.ts index f78139ee8b1..f7337aaac80 100644 --- a/src/redux-middleware/multicursorAlertMiddleware.ts +++ b/src/redux-middleware/multicursorAlertMiddleware.ts @@ -25,7 +25,12 @@ const multicursorAlertMiddleware: ThunkMiddleware = ({ getState, dispatch const numMulticursors = Object.keys(state.multicursors).length // On mobile, show the Command Center when multicursor is active, and hide it when inactive. - if (isTouch) { + // Skip while a multicursor command is executing, since executeCommandWithMulticursor transiently clears and + // restores the multicursors (per-thought setCursor), which would otherwise flicker showCommandCenter + // true → false → true. On iOS WebKit this re-animates the Command Center sliding in from the top. The terminal + // setIsMulticursorExecuting({ value: false }) dispatch flows through here with the settled multicursor count and + // reconciles the final state, preserving the close-on-clear behavior for commands like Delete. + if (isTouch && !state.isMulticursorExecuting) { if (numMulticursors === 0 && state.showCommandCenter) { dispatch(toggleDropdown({ dropDownType: 'commandCenter', value: false })) } else if (numMulticursors > 0 && !state.showCommandCenter && !state.showUndoSlider) { From b0f9dd26ccf76bd088bd0203df70502d1f546bda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 02:43:26 +0000 Subject: [PATCH 03/11] fix(command-center): do not show multicursor alert on mobile during command (#3995 Issue B) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com> --- .../__tests__/multicursorAlertMiddleware.ts | 30 ++++++++++++++++++ .../multicursorAlertMiddleware.ts | 31 +++++++++++-------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts b/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts index 8e3fe2e8ee9..c5b134d30dd 100644 --- a/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts +++ b/src/redux-middleware/__tests__/multicursorAlertMiddleware.ts @@ -4,6 +4,7 @@ import { undoActionCreator as undo } from '../../actions/undo' import { executeCommandWithMulticursor } from '../../commands' import boldCommand from '../../commands/bold' import deleteCommand from '../../commands/delete' +import { AlertType } from '../../constants' import { initialize } from '../../initialize' import store from '../../stores/app' import { addMulticursorAtFirstMatchActionCreator as addMulticursor } from '../../test-helpers/addMulticursorAtFirstMatch' @@ -69,6 +70,35 @@ it('keeps the Command Center open while a multicursor formatting command execute expect(store.getState().showCommandCenter).toBe(true) }) +it('does not show the multicursor alert on mobile while a multicursor command executes', async () => { + await initialize() + + store.dispatch([ + importText({ + text: ` + - A + - B + - C`, + }), + setCursor(['C']), + // open the Command Center by activating a multicursor on the cursor thought (swipe up) + addMulticursor(['C']), + ]) + expect(store.getState().showCommandCenter).toBe(true) + + // apply a formatting command (Bold) across the multicursor + executeCommandWithMulticursor(boldCommand, { store }) + + // Flush the throttled alert dispatch in the middleware. + vi.advanceTimersByTime(1000) + + // On mobile the Command Center—not the alert—reflects the multicursor selection. The "n thoughts selected" alert + // must never be shown on touch, otherwise its auto-dismiss (Alert.tsx clearDelay) clears the multicursors and + // closes the Command Center a few seconds after a formatting command (#3995 Issue B). + expect(store.getState().alert?.alertType).not.toBe(AlertType.MulticursorActive) + expect(store.getState().showCommandCenter).toBe(true) +}) + it('does not show the Command Center when undoing a multicursor delete while the Undo Slider is active', async () => { await initialize() diff --git a/src/redux-middleware/multicursorAlertMiddleware.ts b/src/redux-middleware/multicursorAlertMiddleware.ts index f7337aaac80..d0c313010d0 100644 --- a/src/redux-middleware/multicursorAlertMiddleware.ts +++ b/src/redux-middleware/multicursorAlertMiddleware.ts @@ -25,19 +25,24 @@ const multicursorAlertMiddleware: ThunkMiddleware = ({ getState, dispatch const numMulticursors = Object.keys(state.multicursors).length // On mobile, show the Command Center when multicursor is active, and hide it when inactive. - // Skip while a multicursor command is executing, since executeCommandWithMulticursor transiently clears and - // restores the multicursors (per-thought setCursor), which would otherwise flicker showCommandCenter - // true → false → true. On iOS WebKit this re-animates the Command Center sliding in from the top. The terminal - // setIsMulticursorExecuting({ value: false }) dispatch flows through here with the settled multicursor count and - // reconciles the final state, preserving the close-on-clear behavior for commands like Delete. - if (isTouch && !state.isMulticursorExecuting) { - if (numMulticursors === 0 && state.showCommandCenter) { - dispatch(toggleDropdown({ dropDownType: 'commandCenter', value: false })) - } else if (numMulticursors > 0 && !state.showCommandCenter && !state.showUndoSlider) { - // Do not open the Command Center while the Undo Slider session is active. - // Otherwise undoing/redoing a multicursor command (e.g. delete from the Command Center) restores the - // multicursor, which would re-open the Command Center and dismiss the Undo Slider being used. - dispatch(toggleDropdown({ dropDownType: 'commandCenter', value: true })) + if (isTouch) { + // Skip while a multicursor command is executing, since executeCommandWithMulticursor transiently clears and + // restores the multicursors (per-thought setCursor), which would otherwise flicker showCommandCenter + // true → false → true. On iOS WebKit this re-animates the Command Center sliding in from the top. The terminal + // setIsMulticursorExecuting({ value: false }) dispatch flows through here with the settled multicursor count and + // reconciles the final state, preserving the close-on-clear behavior for commands like Delete. + // The guard is nested inside isTouch (rather than combined into the condition) so that a mobile multicursor + // command never falls through to the desktop alert branch below, which would show the "n thoughts selected" + // alert; that alert auto-dismisses after a few seconds and clears the multicursors, closing the Command Center. + if (!state.isMulticursorExecuting) { + if (numMulticursors === 0 && state.showCommandCenter) { + dispatch(toggleDropdown({ dropDownType: 'commandCenter', value: false })) + } else if (numMulticursors > 0 && !state.showCommandCenter && !state.showUndoSlider) { + // Do not open the Command Center while the Undo Slider session is active. + // Otherwise undoing/redoing a multicursor command (e.g. delete from the Command Center) restores the + // multicursor, which would re-open the Command Center and dismiss the Undo Slider being used. + dispatch(toggleDropdown({ dropDownType: 'commandCenter', value: true })) + } } } // on desktop, show a persistent alert From c8396a4009e2d8db7e74a1da6631860143745685 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 02:47:34 +0000 Subject: [PATCH 04/11] test(e2e): multicursor formatting keeps Command Center open and applies formatting (#3995 Issue B) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com> --- src/e2e/puppeteer/__tests__/multiselect.ts | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/e2e/puppeteer/__tests__/multiselect.ts b/src/e2e/puppeteer/__tests__/multiselect.ts index 5860b9b2dc6..e52376ec3db 100644 --- a/src/e2e/puppeteer/__tests__/multiselect.ts +++ b/src/e2e/puppeteer/__tests__/multiselect.ts @@ -1,5 +1,7 @@ import { KnownDevices } from 'puppeteer' +import click from '../helpers/click' import emulate from '../helpers/emulate' +import exportThoughts from '../helpers/exportThoughts' import longPressThought from '../helpers/longPressThought' import multiselectThoughts from '../helpers/multiselectThoughts' import paste from '../helpers/paste' @@ -57,4 +59,44 @@ describe('mobile only', () => { expect(highlightedBullets.length).toBe(2) }) + + it('keeps the Command Center open and applies formatting when a text formatting command is run on a multiselection', async () => { + await paste(` + - a + - b + - c + `) + + const a = await waitForEditable('a') + const b = await waitForEditable('b') + + await longPressThought(a, { edge: 'right' }) + await longPressThought(b, { edge: 'right' }) + + // Wait for the Command Center to show the multiselection before tapping a formatting command. + await page.waitForFunction( + () => { + const panel = document.querySelector('[data-testid=command-center-panel]') + return panel?.textContent?.includes('2 thoughts selected') ?? false + }, + { timeout: 6000 }, + ) + + // Tap Bold on the toolbar to format the multiselection. + await click('[data-testid="toolbar-icon"][aria-label="Bold"]') + + // Wait past the alert auto-dismiss delay (Alert.tsx clearDelay defaults to 5000ms). Previously a multicursor + // command on mobile fell through to the desktop alert branch and dispatched the "n thoughts selected" alert, + // which auto-dismissed and cleared the multicursors, closing the Command Center a few seconds later (#3995 Issue B). + await new Promise(resolve => setTimeout(resolve, 6000)) + + // The Command Center should still be open with the multiselection intact. + const panelText = await page.$eval('[data-testid=command-center-panel]', el => el.textContent) + expect(panelText).toContain('2 thoughts selected') + + // The formatting should have been applied to both selected thoughts. + const output = await exportThoughts() + expect(output).toContain('**a**') + expect(output).toContain('**b**') + }) }) From f0a279f1af366aa04681bf11f308e82079d2239a Mon Sep 17 00:00:00 2001 From: Bayu Ari Date: Thu, 25 Jun 2026 17:51:20 +0700 Subject: [PATCH 05/11] fix(command-center): apply text formatting and keep toolbar button state in sync for multicursor (#3995) Switch bold/italic/underline/strikethrough commands from formatSelection (document.execCommand) to formatWithTag, which edits the thought value directly. execCommand is a no-op on iOS WebKit when no editable is focused (the multiselect/Command Center flow), so formatting was silently dropped. Keep the toolbar command state in sync after the value-based edit: refresh commandStateStore deterministically in formatWithTag; for a collapsed caret, updateCommandState reads the fresh cursor-thought value instead of the stale post-edit DOM; and skip the cursor-change updateCommandState while a multicursor command is executing so the restored (possibly unselected) cursor does not clobber the selection's state. --- src/actions/formatWithTag.ts | 33 +++++++- src/commands/__tests__/bold.ts | 100 +++++++++++++++++++++++ src/commands/bold.ts | 5 +- src/commands/italic.ts | 5 +- src/commands/strikethrough.ts | 5 +- src/commands/underline.ts | 5 +- src/redux-middleware/updateUrlHistory.ts | 5 +- src/stores/commandStateStore.ts | 4 +- 8 files changed, 148 insertions(+), 14 deletions(-) create mode 100644 src/commands/__tests__/bold.ts diff --git a/src/actions/formatWithTag.ts b/src/actions/formatWithTag.ts index b5e0004303e..0adb1dcc989 100644 --- a/src/actions/formatWithTag.ts +++ b/src/actions/formatWithTag.ts @@ -4,14 +4,26 @@ import Thunk from '../@types/Thunk' import * as selection from '../device/selection' import pathToThought from '../selectors/pathToThought' import thoughtToPath from '../selectors/thoughtToPath' +import commandStateStore from '../stores/commandStateStore' import suppressFocusStore from '../stores/suppressFocus' import getCommandState from '../util/getCommandState' import strip from '../util/strip' import { editThoughtActionCreator as editThought } from './editThought' +/** Maps a FormattingCommand to the HTML tag used to wrap the formatted value. The FormattingCommand value matches the tag name only for `code`, so the others must be mapped explicitly (e.g. bold → b). */ +const formattingCommandTag: Record = { + [FormattingCommand.bold]: 'b', + [FormattingCommand.italic]: 'i', + [FormattingCommand.underline]: 'u', + [FormattingCommand.strikethrough]: 'strike', + [FormattingCommand.code]: 'code', + [FormattingCommand.foreColor]: 'span', + [FormattingCommand.backColor]: 'span', +} + /** Format the browser selection or cursor thought with the provided tag. */ export const formatWithTagActionCreator = - (tag: FormattingCommand): Thunk => + (command: FormattingCommand): Thunk => (dispatch, getState) => { const state = getState() if (!state.cursor) return @@ -20,15 +32,21 @@ export const formatWithTagActionCreator = const simplePath = thoughtToPath(state, thought.id) suppressFocusStore.update(true) + const tag = formattingCommandTag[command] const tagRegExp = new RegExp(`<${tag}[^>]*>|<\/${tag}>`, 'g') const thoughtSelected = (selection.text()?.length === 0 && strip(thought.value).length !== 0) || selection.text()?.length === strip(thought.value).length + // The command state that the toolbar button reads to render its active highlight. It is derived directly from the + // formatted result (not the DOM selection) because formatWithTag edits the thought value via editThought, which + // re-renders asynchronously; reading window.getSelection() here would return the stale, pre-format DOM. + let commandStateSource: string + if (thoughtSelected) { // format the entire thought - const isAllFormatted = getCommandState(thought.value)[tag] + const isAllFormatted = getCommandState(thought.value)[command] const withoutTag = thought.value.replace(tagRegExp, '') const newValue = isAllFormatted ? withoutTag // remove tag @@ -42,11 +60,12 @@ export const formatWithTagActionCreator = force: true, }), ) + commandStateSource = newValue } else { // format the selection const selectedText = selection.html() if (!selectedText) return - const isAllFormatted = getCommandState(selectedText)[tag] + const isAllFormatted = getCommandState(selectedText)[command] const withoutTag = selectedText.replace(tagRegExp, '') const newPart = isAllFormatted ? withoutTag // remove tag @@ -61,6 +80,14 @@ export const formatWithTagActionCreator = force: true, }), ) + // Reflect the state of the formatted selection (newPart), not the whole thought. + commandStateSource = newPart } + + // Refresh the command state store so the toolbar button reflects the new formatting. Unlike formatSelection (which + // uses document.execCommand and relies on the resulting selectionchange event), formatWithTag edits the thought + // value directly, so no selectionchange fires and the button highlight would otherwise go stale. + commandStateStore.update(getCommandState(commandStateSource)) + suppressFocusStore.update(false) } diff --git a/src/commands/__tests__/bold.ts b/src/commands/__tests__/bold.ts new file mode 100644 index 00000000000..e730559c5d7 --- /dev/null +++ b/src/commands/__tests__/bold.ts @@ -0,0 +1,100 @@ +import { importTextActionCreator as importText } from '../../actions/importText' +import { executeCommand, executeCommandWithMulticursor } from '../../commands' +import { HOME_TOKEN } from '../../constants' +import exportContext from '../../selectors/exportContext' +import store from '../../stores/app' +import commandStateStore from '../../stores/commandStateStore' +import { addMulticursorAtFirstMatchActionCreator as addMulticursor } from '../../test-helpers/addMulticursorAtFirstMatch' +import initStore from '../../test-helpers/initStore' +import { setCursorFirstMatchActionCreator as setCursor } from '../../test-helpers/setCursorFirstMatch' +import boldCommand from '../bold' + +beforeEach(initStore) + +describe('bold', () => { + // Regression test for #3995: applying bold to the cursor thought must update the command state store so the toolbar + // button highlights as active. formatWithTag edits the value directly (no document.execCommand selectionchange), so it + // must refresh the command state explicitly. + it('updates the command state so the toolbar button reflects the active formatting', () => { + store.dispatch([ + importText({ + text: ` + - a + - b + `, + }), + setCursor(['a']), + ]) + + executeCommand(boldCommand, { store }) + + expect(commandStateStore.getState().bold).toBe(true) + + // toggling again removes the formatting and deactivates the button state + executeCommand(boldCommand, { store }) + + expect(commandStateStore.getState().bold).toBe(false) + }) + + describe('multicursor', () => { + // Regression test for #3995: applying a text formatting command to a multiselection (e.g. from the Command Center + // on mobile) must format every selected thought. Formatting previously relied on document.execCommand, which is a + // no-op when no editable is focused (real iOS WebKit, and JSDOM in tests), so no formatting was applied. + it('applies bold formatting to all selected thoughts', () => { + store.dispatch([ + importText({ + text: ` + - a + - b + - c + `, + }), + setCursor(['a']), + addMulticursor(['a']), + addMulticursor(['b']), + ]) + + executeCommandWithMulticursor(boldCommand, { store }) + + const exported = exportContext(store.getState(), [HOME_TOKEN], 'text/plain') + expect(exported).toBe(`- __ROOT__ + - **a** + - **b** + - c`) + + // the toolbar button must reflect the active formatting after a multicursor command (#3995) + expect(commandStateStore.getState().bold).toBe(true) + }) + + // Regression test for #3995: when the cursor is on a thought that is NOT part of the multiselection (e.g. the + // selected thoughts are scrolled out of view and an unrelated thought holds the cursor), executeCommandWithMulticursor + // restores the original cursor after formatting. The cursor-change command state update must not run during multicursor + // execution, otherwise it reads the unselected cursor thought's value and deactivates the toolbar button. + it('keeps the command state active when the cursor is on an unselected thought', () => { + store.dispatch([ + importText({ + text: ` + - a + - b + - c + `, + }), + // cursor on c, which is not part of the multiselection {a, b} + setCursor(['c']), + addMulticursor(['a']), + addMulticursor(['b']), + ]) + + executeCommandWithMulticursor(boldCommand, { store }) + + const exported = exportContext(store.getState(), [HOME_TOKEN], 'text/plain') + expect(exported).toBe(`- __ROOT__ + - **a** + - **b** + - c`) + + // the toolbar button must remain active even though the restored cursor (c) is not bold (#3995) + expect(commandStateStore.getState().bold).toBe(true) + }) + }) +}) diff --git a/src/commands/bold.ts b/src/commands/bold.ts index df584bc1f8b..1003c4f01a3 100644 --- a/src/commands/bold.ts +++ b/src/commands/bold.ts @@ -1,5 +1,6 @@ import Command from '../@types/Command' -import { formatSelectionActionCreator as formatSelection } from '../actions/formatSelection' +import FormattingCommand from '../@types/FormattingCommand' +import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/BoldTextIcon' import hasMulticursor from '../selectors/hasMulticursor' import isDocumentEditable from '../util/isDocumentEditable' @@ -17,7 +18,7 @@ const bold: Command = { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) }, exec: dispatch => { - dispatch(formatSelection('bold')) + dispatch(formatWithTag(FormattingCommand.bold)) }, // The isActive logic for formatting commands is handled differently than other commands because it references the CommandStateStore. This can be found in ToolbarButton (isButtonActive) } diff --git a/src/commands/italic.ts b/src/commands/italic.ts index 0753d6be5d8..534398833e9 100644 --- a/src/commands/italic.ts +++ b/src/commands/italic.ts @@ -1,5 +1,6 @@ import Command from '../@types/Command' -import { formatSelectionActionCreator as formatSelection } from '../actions/formatSelection' +import FormattingCommand from '../@types/FormattingCommand' +import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/ItalicTextIcon' import hasMulticursor from '../selectors/hasMulticursor' import isDocumentEditable from '../util/isDocumentEditable' @@ -17,7 +18,7 @@ const italic: Command = { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) }, exec: dispatch => { - dispatch(formatSelection('italic')) + dispatch(formatWithTag(FormattingCommand.italic)) }, // The isActive logic for formatting commands is handled differently than other commands because it references the CommandStateStore. This can be found in ToolbarButton (isButtonActive) } diff --git a/src/commands/strikethrough.ts b/src/commands/strikethrough.ts index 9c6470e69a9..1ba93e3cc40 100644 --- a/src/commands/strikethrough.ts +++ b/src/commands/strikethrough.ts @@ -1,5 +1,6 @@ import Command from '../@types/Command' -import { formatSelectionActionCreator as formatSelection } from '../actions/formatSelection' +import FormattingCommand from '../@types/FormattingCommand' +import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/StrikethroughIcon' import hasMulticursor from '../selectors/hasMulticursor' import isDocumentEditable from '../util/isDocumentEditable' @@ -18,7 +19,7 @@ const strikethrough: Command = { }, exec: (dispatch, getState, e) => { e.preventDefault() - dispatch(formatSelection('strikethrough')) + dispatch(formatWithTag(FormattingCommand.strikethrough)) }, // The isActive logic for formatting commands is handled differently than other commands because it references the CommandStateStore. This can be found in ToolbarButton (isButtonActive) } diff --git a/src/commands/underline.ts b/src/commands/underline.ts index 3aad0aa8a15..6bb15767201 100644 --- a/src/commands/underline.ts +++ b/src/commands/underline.ts @@ -1,5 +1,6 @@ import Command from '../@types/Command' -import { formatSelectionActionCreator as formatSelection } from '../actions/formatSelection' +import FormattingCommand from '../@types/FormattingCommand' +import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/UnderlineIcon' import hasMulticursor from '../selectors/hasMulticursor' import isDocumentEditable from '../util/isDocumentEditable' @@ -19,7 +20,7 @@ const underline: Command = { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) }, exec: dispatch => { - dispatch(formatSelection('underline')) + dispatch(formatWithTag(FormattingCommand.underline)) }, // The isActive logic for formatting commands is handled differently than other commands because it references the CommandStateStore. This can be found in ToolbarButton (isButtonActive) } diff --git a/src/redux-middleware/updateUrlHistory.ts b/src/redux-middleware/updateUrlHistory.ts index de3207c5a4a..a263da07433 100644 --- a/src/redux-middleware/updateUrlHistory.ts +++ b/src/redux-middleware/updateUrlHistory.ts @@ -136,8 +136,11 @@ const updateUrlHistoryMiddleware: ThunkMiddleware = ({ getState }) => { // Update the command state whenever the cursor changes. // Otherwise the command state will not update when the cursor is moved with no selection (mobile only, when the keyboard is down), since updateCommandState is otherwise only called on selection change. + // Skip while a multicursor command is executing: executeCommandWithMulticursor moves the cursor to each selected + // thought in turn and then restores the original cursor, which may be an unselected thought. Updating the command + // state from those transient/restored cursors would clobber the state that the command set for the selection (#3995). const cursor = getState().cursor - if (!equalPath(cursor, cursorPrev)) { + if (!equalPath(cursor, cursorPrev) && !getState().isMulticursorExecuting) { updateCommandState() } cursorPrev = cursor diff --git a/src/stores/commandStateStore.ts b/src/stores/commandStateStore.ts index 8ad2d05be9a..d5975c847d6 100644 --- a/src/stores/commandStateStore.ts +++ b/src/stores/commandStateStore.ts @@ -29,12 +29,12 @@ export const resetCommandState = () => { }) } -/** Updates the command state to the current selection/thought. If there is an active selection, this uses document.queryCommandState to get the command state from the DOM. This detects a formatting style that has been enabled, but not yet entered (i.e. the next character typed will be bold). If there is no selection, this parses the cursor thought's value and sets a formatting state only if it applies to the entire thought. */ +/** Updates the command state to the current selection/thought. If there is a non-collapsed selection within a thought, this parses the selected HTML to detect a formatting style that applies to the selection. Otherwise (no selection or a collapsed caret) it parses the cursor thought's value and sets a formatting state only if it applies to the entire thought. A collapsed caret deliberately uses the cursor thought's value (from state) rather than the DOM, since the DOM may be stale immediately after a programmatic edit (e.g. formatWithTag) until React re-renders. */ export const updateCommandState = () => { const state = store.getState() if (!state.cursor) return const action = - selection.isActive() && selection.isThought() + selection.isActive() && selection.isThought() && !selection.isCollapsed() ? getCommandState(selection.html() ?? '') : getCommandState(pathToThought(state, state.cursor)?.value ?? '') commandStateStore.update(action) From 1a4266d17d2385a4c673c7a648f30d6764b1cd50 Mon Sep 17 00:00:00 2001 From: Bayu Ari Date: Thu, 25 Jun 2026 18:20:53 +0700 Subject: [PATCH 06/11] Scroll topmost selected thought into view after multicursor formatting (#3995) When a multicursor formatting command (bold/italic/underline/strikethrough/code) completes while the selected thoughts are scrolled offscreen (e.g. when the Command Center is onscreen), snap the topmost selected thought into view so the formatted thoughts remain visible. Reuses scrollCursorIntoView, which no-ops when the thought is already in view, so it does not snap when the selection is in focus. --- src/commands/bold.ts | 3 ++- src/commands/code.ts | 3 ++- src/commands/italic.ts | 3 ++- src/commands/strikethrough.ts | 3 ++- src/commands/underline.ts | 2 ++ src/components/LayoutTree.tsx | 17 +++++++++++++++++ src/stores/scrollMulticursorIntoView.ts | 17 +++++++++++++++++ 7 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 src/stores/scrollMulticursorIntoView.ts diff --git a/src/commands/bold.ts b/src/commands/bold.ts index 1003c4f01a3..02bb6fe580f 100644 --- a/src/commands/bold.ts +++ b/src/commands/bold.ts @@ -3,6 +3,7 @@ import FormattingCommand from '../@types/FormattingCommand' import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/BoldTextIcon' import hasMulticursor from '../selectors/hasMulticursor' +import { scrollMulticursorIntoViewOnComplete } from '../stores/scrollMulticursorIntoView' import isDocumentEditable from '../util/isDocumentEditable' /** Toggles formatting of the current browser selection as bold. If there is no selection, formats the entire thought. */ @@ -11,7 +12,7 @@ const bold: Command = { label: 'Bold', description: 'Bolds the current thought or selected text.', descriptionInverse: 'Removes bold from the current thought or selected text.', - multicursor: true, + multicursor: { onComplete: scrollMulticursorIntoViewOnComplete }, svg: Icon, keyboard: { key: 'b', meta: true }, canExecute: state => { diff --git a/src/commands/code.ts b/src/commands/code.ts index 80125bcf5c9..2e9a9952627 100644 --- a/src/commands/code.ts +++ b/src/commands/code.ts @@ -3,6 +3,7 @@ import FormattingCommand from '../@types/FormattingCommand' import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/CodeIcon' import hasMulticursor from '../selectors/hasMulticursor' +import { scrollMulticursorIntoViewOnComplete } from '../stores/scrollMulticursorIntoView' import isDocumentEditable from '../util/isDocumentEditable' /** Toggles formatting of the current browser selection as code. If there is no selection, formats the entire thought. */ @@ -11,7 +12,7 @@ const codeCommand: Command = { label: 'Code', description: 'Formats the current thought or selected text as code.', descriptionInverse: 'Removes code formatting from the current thought or selected text.', - multicursor: true, + multicursor: { onComplete: scrollMulticursorIntoViewOnComplete }, svg: Icon, keyboard: { key: 'k', meta: true }, canExecute: state => { diff --git a/src/commands/italic.ts b/src/commands/italic.ts index 534398833e9..7e2af05252d 100644 --- a/src/commands/italic.ts +++ b/src/commands/italic.ts @@ -3,6 +3,7 @@ import FormattingCommand from '../@types/FormattingCommand' import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/ItalicTextIcon' import hasMulticursor from '../selectors/hasMulticursor' +import { scrollMulticursorIntoViewOnComplete } from '../stores/scrollMulticursorIntoView' import isDocumentEditable from '../util/isDocumentEditable' /** Toggles formatting of the current browser selection as italic. If there is no selection, formats the entire thought. */ @@ -13,7 +14,7 @@ const italic: Command = { descriptionInverse: 'Removes italics from the current thought or selected text. So much for being special.', svg: Icon, keyboard: { key: 'i', meta: true }, - multicursor: true, + multicursor: { onComplete: scrollMulticursorIntoViewOnComplete }, canExecute: state => { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) }, diff --git a/src/commands/strikethrough.ts b/src/commands/strikethrough.ts index 1ba93e3cc40..6efc4caa876 100644 --- a/src/commands/strikethrough.ts +++ b/src/commands/strikethrough.ts @@ -3,6 +3,7 @@ import FormattingCommand from '../@types/FormattingCommand' import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/StrikethroughIcon' import hasMulticursor from '../selectors/hasMulticursor' +import { scrollMulticursorIntoViewOnComplete } from '../stores/scrollMulticursorIntoView' import isDocumentEditable from '../util/isDocumentEditable' /** Toggles formatting of the current browser selection as strikethrough. If there is no selection, formats the entire thought. */ @@ -13,7 +14,7 @@ const strikethrough: Command = { descriptionInverse: 'Removes strikethrough from the current thought or selected text.', svg: Icon, keyboard: { key: 's', meta: true }, - multicursor: true, + multicursor: { onComplete: scrollMulticursorIntoViewOnComplete }, canExecute: state => { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) }, diff --git a/src/commands/underline.ts b/src/commands/underline.ts index 6bb15767201..ffe849c9f93 100644 --- a/src/commands/underline.ts +++ b/src/commands/underline.ts @@ -3,6 +3,7 @@ import FormattingCommand from '../@types/FormattingCommand' import { formatWithTagActionCreator as formatWithTag } from '../actions/formatWithTag' import Icon from '../components/icons/UnderlineIcon' import hasMulticursor from '../selectors/hasMulticursor' +import { scrollMulticursorIntoViewOnComplete } from '../stores/scrollMulticursorIntoView' import isDocumentEditable from '../util/isDocumentEditable' /** Toggles formatting of the current browser selection as underline. If there is no selection, formats the entire thought. */ @@ -15,6 +16,7 @@ const underline: Command = { keyboard: { key: 'u', meta: true }, multicursor: { preventSetCursor: true, + onComplete: scrollMulticursorIntoViewOnComplete, }, canExecute: state => { return isDocumentEditable() && (!!state.cursor || hasMulticursor(state)) diff --git a/src/components/LayoutTree.tsx b/src/components/LayoutTree.tsx index fdfde9e8f42..43480729273 100644 --- a/src/components/LayoutTree.tsx +++ b/src/components/LayoutTree.tsx @@ -7,14 +7,18 @@ import Index from '../@types/IndexType' import ThoughtId from '../@types/ThoughtId' import { isTouch } from '../browser' import { CONTENT_BOX_PADDING_LEFT, LongPressState } from '../constants' +import scrollCursorIntoView from '../device/scrollCursorIntoView' import testFlags from '../e2e/testFlags' import usePositionedThoughts from '../hooks/usePositionedThoughts' import useSizeTracking from '../hooks/useSizeTracking' import fauxCaretTreeProvider from '../recipes/fauxCaretTreeProvider' import { hasChildren } from '../selectors/getChildren' +import isMulticursorPath from '../selectors/isMulticursorPath' import linearizeTree from '../selectors/linearizeTree' import nextSibling from '../selectors/nextSibling' +import store from '../stores/app' import reactMinistore from '../stores/react-ministore' +import scrollMulticursorIntoViewStore from '../stores/scrollMulticursorIntoView' import scrollTopStore from '../stores/scrollTop' import viewportStore from '../stores/viewport' import head from '../util/head' @@ -247,6 +251,19 @@ const LayoutTree = () => { const cursorThoughtPositionedIndex = treeThoughtsPositioned.findIndex(thought => thought.isCursor) const cursorThoughtPositioned = treeThoughtsPositioned[cursorThoughtPositionedIndex] + // After a multicursor formatting command completes, scroll the topmost selected thought into view if it is offscreen. + // The selected thoughts may be scrolled out of view (e.g. when the command center is onscreen), so snap to the topmost + // selected thought to keep the formatted thoughts visible. scrollCursorIntoView is a no-op when the thought is already + // in view, so this does not snap when the selection is already in focus. See #3995. + scrollMulticursorIntoViewStore.useEffect(() => { + const stateNow = store.getState() + // treeThoughtsPositioned is in document order, so the first selected thought is the topmost one. + const topmost = treeThoughtsPositioned.find(thought => isMulticursorPath(stateNow, thought.path)) + if (!topmost) return + // Defer to the next tick so any layout changes from the formatting command have been committed. + setTimeout(() => scrollCursorIntoView(topmost.y, topmost.height)) + }) + // The indentDepth multipicand (0.9) causes the horizontal counter-indentation to fall short of the actual indentation, causing a progressive shifting right as the user navigates deeper. This provides an additional cue for the user's depth, which is helpful when autofocus obscures the actual depth, but it must stay small otherwise the thought width becomes too small. // The indentCursorAncestorTables multipicand (0.5) is smaller, since animating over by the entire width of column 1 is too abrupt. // (The same multiplicand is applied to the vertical translation that crops hidden thoughts above the cursor.) diff --git a/src/stores/scrollMulticursorIntoView.ts b/src/stores/scrollMulticursorIntoView.ts new file mode 100644 index 00000000000..a8953cfbfef --- /dev/null +++ b/src/stores/scrollMulticursorIntoView.ts @@ -0,0 +1,17 @@ +import reactMinistore from './react-ministore' + +/** + * Pulse store that signals the layout to scroll the topmost selected thought into view. + * + * It is incremented after a multicursor formatting command completes (see the formatting commands' multicursor.onComplete). + * The actual scroll is performed in LayoutTree, which is the only place that has the positioned thoughts (y/height). + * See #3995. + */ +const scrollMulticursorIntoViewStore = reactMinistore(0) + +/** A multicursor onComplete handler that triggers a scroll of the topmost selected thought into view (only if it is offscreen). Used by the formatting commands. */ +export const scrollMulticursorIntoViewOnComplete = () => { + scrollMulticursorIntoViewStore.update(n => n + 1) +} + +export default scrollMulticursorIntoViewStore From 0041b594a7419e2b803884210a87ff030c24dd62 Mon Sep 17 00:00:00 2001 From: Bayu Ari Date: Thu, 25 Jun 2026 18:30:29 +0700 Subject: [PATCH 07/11] Read note formatting from the DOM in updateCommandState when a note is focused (#3995) The collapsed-caret branch added for the multicursor formatting fix reads the cursor thought's value, but a focused note is a separate thought from the cursor thought, so its color/formatting was no longer detected. This broke toggling a note's color off (the swatch re-applied instead of removing). Read selection.html() when noteFocus is set, restoring the pre-existing note behavior while preserving the collapsed-caret fix for regular thoughts. --- src/stores/commandStateStore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stores/commandStateStore.ts b/src/stores/commandStateStore.ts index d5975c847d6..2c435559c42 100644 --- a/src/stores/commandStateStore.ts +++ b/src/stores/commandStateStore.ts @@ -29,12 +29,12 @@ export const resetCommandState = () => { }) } -/** Updates the command state to the current selection/thought. If there is a non-collapsed selection within a thought, this parses the selected HTML to detect a formatting style that applies to the selection. Otherwise (no selection or a collapsed caret) it parses the cursor thought's value and sets a formatting state only if it applies to the entire thought. A collapsed caret deliberately uses the cursor thought's value (from state) rather than the DOM, since the DOM may be stale immediately after a programmatic edit (e.g. formatWithTag) until React re-renders. */ +/** Updates the command state to the current selection/thought. If there is a non-collapsed selection within a thought, this parses the selected HTML to detect a formatting style that applies to the selection. Otherwise (no selection or a collapsed caret) it parses the cursor thought's value and sets a formatting state only if it applies to the entire thought. A collapsed caret deliberately uses the cursor thought's value (from state) rather than the DOM, since the DOM may be stale immediately after a programmatic edit (e.g. formatWithTag) until React re-renders. The exception is when a note is focused: the note is a separate thought from the cursor thought, so its formatting can only be read from the DOM (selection.html()), not from the cursor thought's value. */ export const updateCommandState = () => { const state = store.getState() if (!state.cursor) return const action = - selection.isActive() && selection.isThought() && !selection.isCollapsed() + selection.isActive() && selection.isThought() && (!selection.isCollapsed() || state.noteFocus) ? getCommandState(selection.html() ?? '') : getCommandState(pathToThought(state, state.cursor)?.value ?? '') commandStateStore.update(action) From 2ab75028b030f29d7ea2439ac297d0b15c0ec126 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 26 Jun 2026 03:30:39 +0000 Subject: [PATCH 08/11] fix(scroll): account for Command Center as bottom obstruction in scrollCursorIntoView (#3995 Issue G) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com> --- src/device/__tests__/scrollCursorIntoView.ts | 65 ++++++++++++++++++++ src/device/scrollCursorIntoView.ts | 15 ++++- 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 src/device/__tests__/scrollCursorIntoView.ts diff --git a/src/device/__tests__/scrollCursorIntoView.ts b/src/device/__tests__/scrollCursorIntoView.ts new file mode 100644 index 00000000000..126ff683740 --- /dev/null +++ b/src/device/__tests__/scrollCursorIntoView.ts @@ -0,0 +1,65 @@ +import viewportStore from '../../stores/viewport' +import scrollCursorIntoView from '../scrollCursorIntoView' + +/** Viewport height used for the tests (approximately iPhone 15 Pro). */ +const VIEWPORT_HEIGHT = 852 + +/** Appends an element with the given attributes to the document body and stubs its getBoundingClientRect. */ +const appendElement = (attributes: Record, rect: { top: number; height: number }): HTMLElement => { + const el = document.createElement('div') + Object.entries(attributes).forEach(([key, value]) => el.setAttribute(key, value)) + el.getBoundingClientRect = () => + ({ + top: rect.top, + bottom: rect.top + rect.height, + height: rect.height, + left: 0, + right: 0, + width: 0, + x: 0, + y: rect.top, + toJSON: () => ({}), + }) as DOMRect + document.body.appendChild(el) + return el +} + +let scrollToSpy: ReturnType + +beforeEach(() => { + document.body.innerHTML = '' + + // mock the viewport + Object.defineProperty(window, 'innerHeight', { value: VIEWPORT_HEIGHT, configurable: true }) + Object.defineProperty(window, 'visualViewport', { value: { height: VIEWPORT_HEIGHT }, configurable: true }) + Object.defineProperty(window, 'scrollY', { value: 0, configurable: true, writable: true }) + viewportStore.update({ layoutTreeTop: 0 }) + + scrollToSpy = vi.fn() + window.scrollTo = scrollToSpy as typeof window.scrollTo + + // toolbar at the top, navbar at the bottom + appendElement({ id: 'toolbar' }, { top: 0, height: 50 }) + appendElement({ 'aria-label': 'nav' }, { top: VIEWPORT_HEIGHT - 50, height: 50 }) +}) + +it('does not scroll a thought that is already within the visible viewport', () => { + // a thought in the middle of the screen, with no Command Center open + scrollCursorIntoView(400, 40) + expect(scrollToSpy).not.toHaveBeenCalled() +}) + +it('scrolls a selected thought out from behind the Command Center', () => { + // The Command Center sheet covers the bottom 70% of the screen during a multiselection. + appendElement({ 'data-testid': 'command-menu-panel' }, { top: VIEWPORT_HEIGHT * 0.3, height: VIEWPORT_HEIGHT * 0.7 }) + + // A thought at y=400 is "in view" by the navbar-only measurement, but is hidden behind the Command Center. + // It should be scrolled up so that it lands above the Command Center. See #3995 Issue G. + scrollCursorIntoView(400, 40) + + expect(scrollToSpy).toHaveBeenCalledTimes(1) + const top = scrollToSpy.mock.calls[0][0].top as number + // after scrolling, the thought's bottom should be above the top of the Command Center + const thoughtBottomAfterScroll = 400 - top + 40 + expect(thoughtBottomAfterScroll).toBeLessThanOrEqual(VIEWPORT_HEIGHT * 0.3) +}) diff --git a/src/device/scrollCursorIntoView.ts b/src/device/scrollCursorIntoView.ts index fe1dd9c3a87..6d8e8ab9da5 100644 --- a/src/device/scrollCursorIntoView.ts +++ b/src/device/scrollCursorIntoView.ts @@ -32,8 +32,17 @@ const scrollIntoViewIfNeeded = (y: number, height: number) => { const toolbarRect = document.getElementById('toolbar')?.getBoundingClientRect() const toolbarBottom = toolbarRect ? toolbarRect.bottom : 0 const navbarRect = document.querySelector('[aria-label="nav"]')?.getBoundingClientRect() + + // The Command Center sheet covers the bottom portion of the screen during a multiselection. + // Treat the area it covers as a bottom obstruction (alongside the navbar) so that selected thoughts + // are scrolled above it rather than left hidden behind it. Without this, a selected thought in the + // covered area is considered "in view" and never scrolled out from behind the Command Center. See #3995 Issue G. + const commandCenterRect = document.querySelector('[data-testid="command-menu-panel"]')?.getBoundingClientRect() + const commandCenterHeight = commandCenterRect ? Math.max(0, visualViewportHeight - commandCenterRect.top) : 0 + const bottomObstruction = Math.max(navbarRect?.height ?? 0, commandCenterHeight) + const isAboveViewport = yViewport < toolbarBottom - const isBelowViewport = yViewport + height > visualViewportHeight - (navbarRect?.height ?? 0) + const isBelowViewport = yViewport + height > visualViewportHeight - bottomObstruction if (!isAboveViewport && !isBelowViewport) return @@ -41,10 +50,10 @@ const scrollIntoViewIfNeeded = (y: number, height: number) => { // Therefore, we need to calculate the scroll position ourselves // leave a margin between the element and the viewport edge equal to half the element's height - // add offset to account for the navbar height and prevent scrolled to elements from being hidden below + // add offset to account for the bottom obstruction (navbar or Command Center) and prevent scrolled to elements from being hidden below const scrollYNew = isAboveViewport ? yDocument - (toolbarRect?.height ?? 0) - height / 2 - : yDocument - visualViewportHeight + height * 1.5 + (navbarRect?.height ?? 0) + : yDocument - visualViewportHeight + height * 1.5 + bottomObstruction // scroll to 1 instead of 0 // otherwise Mobile Safari scrolls to the top after MultiGesture From 09197ca8422b46ca142c64265eff3524ae6955d1 Mon Sep 17 00:00:00 2001 From: Bayu Ari Date: Fri, 26 Jun 2026 18:22:04 +0700 Subject: [PATCH 09/11] fix(scroll): keep bottom selected thought clear of Command Center blur without re-scrolling visible thoughts (#3995 Issue F/G) --- src/device/__tests__/scrollCursorIntoView.ts | 27 +++++++++++++++----- src/device/scrollCursorIntoView.ts | 21 ++++++++++++++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/device/__tests__/scrollCursorIntoView.ts b/src/device/__tests__/scrollCursorIntoView.ts index 126ff683740..81a1f9cc0c9 100644 --- a/src/device/__tests__/scrollCursorIntoView.ts +++ b/src/device/__tests__/scrollCursorIntoView.ts @@ -49,17 +49,32 @@ it('does not scroll a thought that is already within the visible viewport', () = expect(scrollToSpy).not.toHaveBeenCalled() }) -it('scrolls a selected thought out from behind the Command Center', () => { +it('scrolls a selected thought above the Command Center blur band so its text is readable', () => { // The Command Center sheet covers the bottom 70% of the screen during a multiselection. - appendElement({ 'data-testid': 'command-menu-panel' }, { top: VIEWPORT_HEIGHT * 0.3, height: VIEWPORT_HEIGHT * 0.7 }) + const commandCenterTop = VIEWPORT_HEIGHT * 0.3 + appendElement({ 'data-testid': 'command-menu-panel' }, { top: commandCenterTop, height: VIEWPORT_HEIGHT * 0.7 }) - // A thought at y=400 is "in view" by the navbar-only measurement, but is hidden behind the Command Center. - // It should be scrolled up so that it lands above the Command Center. See #3995 Issue G. + // The blur element fades in over a 110px band above the solid panel top; a thought left within that band + // would be blurred and unreadable. The thought should be scrolled fully above the blur band. See #3995 Issue G. + const BLUR_FALLOFF = 110 scrollCursorIntoView(400, 40) expect(scrollToSpy).toHaveBeenCalledTimes(1) const top = scrollToSpy.mock.calls[0][0].top as number - // after scrolling, the thought's bottom should be above the top of the Command Center + // after scrolling, the thought's bottom should be above the top of the blur band (panel top - blur falloff) const thoughtBottomAfterScroll = 400 - top + 40 - expect(thoughtBottomAfterScroll).toBeLessThanOrEqual(VIEWPORT_HEIGHT * 0.3) + expect(thoughtBottomAfterScroll).toBeLessThanOrEqual(commandCenterTop - BLUR_FALLOFF) +}) + +it('does not re-scroll a thought already visible above the Command Center panel (no jump)', () => { + // The Command Center sheet covers the bottom 70% of the screen during a multiselection. + const commandCenterTop = VIEWPORT_HEIGHT * 0.3 + appendElement({ 'data-testid': 'command-menu-panel' }, { top: commandCenterTop, height: VIEWPORT_HEIGHT * 0.7 }) + + // A thought whose bottom sits just above the solid panel top is visible and must not be scrolled, otherwise + // snapping the topmost selected thought into view jumps the page. The blur falloff is only applied to the + // scroll *target*, not to the "needs scrolling" detection, so this thought stays put. See #3995 Issue F. + scrollCursorIntoView(commandCenterTop - 60, 40) + + expect(scrollToSpy).not.toHaveBeenCalled() }) diff --git a/src/device/scrollCursorIntoView.ts b/src/device/scrollCursorIntoView.ts index 6d8e8ab9da5..564c0a5be57 100644 --- a/src/device/scrollCursorIntoView.ts +++ b/src/device/scrollCursorIntoView.ts @@ -2,6 +2,17 @@ import { isSafari, isTouch } from '../browser' import { PREVENT_AUTOSCROLL_TIMEOUT, isPreventAutoscrollInProgress } from '../device/preventAutoscroll' import viewportStore from '../stores/viewport' +/** + * The height (in px) of the Command Center's progressive blur falloff that extends above the solid panel. + * The blur element is taller than the panel by this amount (`blurHeight = sheetHeight + 110`) and fades in + * over this band via a mask gradient, so a thought scrolled to just above the panel top would still be + * blurred. It is added to the *scroll target* obstruction (not the "needs scrolling" detection) so that a + * thought scrolled out from behind the panel lands clear of the blur band, while thoughts already visible + * above the panel are not needlessly re-scrolled. + * Keep in sync with the `110` used in CommandCenter.tsx (blurHeight and the blur mask gradient). + */ +const COMMAND_CENTER_BLUR_FALLOFF = 110 + /** Scrolls the minimum amount necessary to move the viewport so that it includes the element. */ const scrollIntoViewIfNeeded = (y: number, height: number) => { // preventAutoscroll works by briefly increasing the element's height, which breaks isElementInViewport. @@ -39,7 +50,15 @@ const scrollIntoViewIfNeeded = (y: number, height: number) => { // covered area is considered "in view" and never scrolled out from behind the Command Center. See #3995 Issue G. const commandCenterRect = document.querySelector('[data-testid="command-menu-panel"]')?.getBoundingClientRect() const commandCenterHeight = commandCenterRect ? Math.max(0, visualViewportHeight - commandCenterRect.top) : 0 + // The obstruction used to decide *whether* a thought needs scrolling is the actual covered area (panel + navbar). + // Using only the panel here (not the blur falloff) keeps thoughts that are already visible above the panel from + // being needlessly re-scrolled, which would cause a jump (e.g. when snapping the topmost selected thought into + // view above the viewport). See #3995 Issue F. const bottomObstruction = Math.max(navbarRect?.height ?? 0, commandCenterHeight) + // The obstruction used to decide *where* to scroll extends the Command Center by its blur falloff, so a thought + // scrolled out from behind the panel lands above the blur band (its text is clear) rather than within it. See #3995 Issue G. + const commandCenterTargetHeight = commandCenterHeight > 0 ? commandCenterHeight + COMMAND_CENTER_BLUR_FALLOFF : 0 + const bottomObstructionTarget = Math.max(navbarRect?.height ?? 0, commandCenterTargetHeight) const isAboveViewport = yViewport < toolbarBottom const isBelowViewport = yViewport + height > visualViewportHeight - bottomObstruction @@ -53,7 +72,7 @@ const scrollIntoViewIfNeeded = (y: number, height: number) => { // add offset to account for the bottom obstruction (navbar or Command Center) and prevent scrolled to elements from being hidden below const scrollYNew = isAboveViewport ? yDocument - (toolbarRect?.height ?? 0) - height / 2 - : yDocument - visualViewportHeight + height * 1.5 + bottomObstruction + : yDocument - visualViewportHeight + height * 1.5 + bottomObstructionTarget // scroll to 1 instead of 0 // otherwise Mobile Safari scrolls to the top after MultiGesture From b7d3189d9bec98a9a4c027005f80637b41245372 Mon Sep 17 00:00:00 2001 From: Bayu Ari Date: Fri, 26 Jun 2026 18:22:15 +0700 Subject: [PATCH 10/11] fix(scroll): prevent autoscroll when applying formatting to a Select All multiselection (#3995 Issue H) --- src/components/LayoutTree.tsx | 5 +++ src/e2e/puppeteer/__tests__/scroll.ts | 56 +++++++++++++++++++++++++++ src/hooks/useScrollCursorIntoView.ts | 22 ++++++++++- 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/components/LayoutTree.tsx b/src/components/LayoutTree.tsx index 43480729273..4de8be4cd1a 100644 --- a/src/components/LayoutTree.tsx +++ b/src/components/LayoutTree.tsx @@ -13,6 +13,7 @@ import usePositionedThoughts from '../hooks/usePositionedThoughts' import useSizeTracking from '../hooks/useSizeTracking' import fauxCaretTreeProvider from '../recipes/fauxCaretTreeProvider' import { hasChildren } from '../selectors/getChildren' +import isAllSelected from '../selectors/isAllSelected' import isMulticursorPath from '../selectors/isMulticursorPath' import linearizeTree from '../selectors/linearizeTree' import nextSibling from '../selectors/nextSibling' @@ -257,6 +258,10 @@ const LayoutTree = () => { // in view, so this does not snap when the selection is already in focus. See #3995. scrollMulticursorIntoViewStore.useEffect(() => { const stateNow = store.getState() + // When all thoughts are selected (Select All), every thought is already "in focus", so snapping to the topmost + // selected thought serves no purpose and instead causes a disruptive autoscroll (up, and on mobile up-and-down as + // the Command Center slides in). Skip the scroll entirely in that case. See #3995 Issue H. + if (isAllSelected(stateNow)) return // treeThoughtsPositioned is in document order, so the first selected thought is the topmost one. const topmost = treeThoughtsPositioned.find(thought => isMulticursorPath(stateNow, thought.path)) if (!topmost) return diff --git a/src/e2e/puppeteer/__tests__/scroll.ts b/src/e2e/puppeteer/__tests__/scroll.ts index bbceae4c090..afc4a848e75 100644 --- a/src/e2e/puppeteer/__tests__/scroll.ts +++ b/src/e2e/puppeteer/__tests__/scroll.ts @@ -1,5 +1,6 @@ import { WindowEm } from '../../../initialize' import clickThought from '../helpers/clickThought' +import command from '../helpers/command' import getEditingText from '../helpers/getEditingText' import paste from '../helpers/paste' import refresh from '../helpers/refresh' @@ -206,3 +207,58 @@ describe('autocrop', () => { expect(yDiff).toBeLessThan(1) }) }) + +describe('scrollMulticursorIntoView', () => { + it('does not autoscroll when applying formatting to a Select All multiselection', async () => { + // A short viewport guarantees the thoughts overflow so there is room to (incorrectly) scroll. See #3995 Issue H. + await page.setViewport({ width: 800, height: 500 }) + + const importText = ` + - 1 + - 2 + - 3 + - 4 + - 5 + - 6 + - 7 + - 8 + - 9 + - 10 + - 11 + - 12 + - 13 + - 14 + - 15 + - 16 + - 17 + - 18 + - 19 + - 20 + ` + await paste(importText) + await clickThought('20') + await waitForEditable('20') + + // Scroll to the bottom so the topmost selected thought is far above the viewport, maximizing any (incorrect) scroll. + await page.evaluate(() => { + const _em = window.em as WindowEm + _em.testFlags.throttledScrollCursorIntoView?.cancel() + window.scrollTo({ top: document.body.scrollHeight, behavior: 'instant' }) + }) + await new Promise(resolve => setTimeout(resolve, 100)) + + const scrollYBefore = await page.evaluate(() => Math.round(window.scrollY)) + // Sanity check: the page must actually be scrolled, otherwise there would be nothing to assert. + expect(scrollYBefore).toBeGreaterThan(0) + + // Select All, then apply a formatting command. Neither should scroll the page. See #3995 Issue H. + await command('selectAll') + await command('bold') + + // Wait for any (incorrect) scroll animation to settle. + await new Promise(resolve => setTimeout(resolve, 1000)) + + const scrollYAfter = await page.evaluate(() => Math.round(window.scrollY)) + expect(scrollYAfter).toBe(scrollYBefore) + }) +}) diff --git a/src/hooks/useScrollCursorIntoView.ts b/src/hooks/useScrollCursorIntoView.ts index 2783481988c..54d300b9d4b 100644 --- a/src/hooks/useScrollCursorIntoView.ts +++ b/src/hooks/useScrollCursorIntoView.ts @@ -2,6 +2,8 @@ import { throttle } from 'lodash' import { useEffect, useRef } from 'react' import scrollCursorIntoView from '../device/scrollCursorIntoView' import testFlags from '../e2e/testFlags' +import isAllSelected from '../selectors/isAllSelected' +import store from '../stores/app' import editingValueStore from '../stores/editingValue' const throttledScrollCursorIntoView = throttle((y: number, height: number) => scrollCursorIntoView(y, height), 400) @@ -9,6 +11,14 @@ const throttledScrollCursorIntoView = throttle((y: number, height: number) => sc // Expose the throttled function so that tests can cancel its pending trailing call before asserting on the scroll position. testFlags.throttledScrollCursorIntoView = throttledScrollCursorIntoView +/** Returns true if every thought at the cursor's level is selected (Select All). In that case the whole level is + * already "in focus", so autoscrolling the cursor when a multicursor formatting command rewrites its value would be a + * disruptive, pointless jump. See #3995 Issue H. */ +const isSelectAllActive = () => { + const state = store.getState() + return Object.keys(state.multicursors).length > 0 && isAllSelected(state) +} + /** Call scrollCursorIntoView when the y position of its container changes, or when the editing value changes. */ const useScrollCursorIntoView = (y: number, height: number) => { const sizeRef = useRef({ y, height }) @@ -27,10 +37,18 @@ const useScrollCursorIntoView = (y: number, height: number) => { * Since sizeRef is an object, it is possible to mutate its properties within the existing closure after * React's render cycle runs and processes the effect above. That's why setTimeout is necessary here (#3083). */ - setTimeout(() => throttledScrollCursorIntoView(sizeRef.current.y, sizeRef.current.height)) + setTimeout(() => { + // Do not autoscroll while a Select All multicursor format rewrites the cursor's value. See #3995 Issue H. + if (isSelectAllActive()) return + throttledScrollCursorIntoView(sizeRef.current.y, sizeRef.current.height) + }) }) - useEffect(() => scrollCursorIntoView(y, height), [height, y]) + useEffect(() => { + // Do not autoscroll while a Select All multicursor format reflows the cursor's position. See #3995 Issue H. + if (isSelectAllActive()) return + scrollCursorIntoView(y, height) + }, [height, y]) } export default useScrollCursorIntoView From c9e5f60ae3e032c7b6b1dbb7bcc289f26e8f41bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 05:06:26 +0000 Subject: [PATCH 11/11] fix(scroll): suppress cursor-follow autoscroll during any multiselection, not just Select All (#3995 Issue F) Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com> --- src/hooks/useScrollCursorIntoView.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/hooks/useScrollCursorIntoView.ts b/src/hooks/useScrollCursorIntoView.ts index 54d300b9d4b..d9b2b2eb6e4 100644 --- a/src/hooks/useScrollCursorIntoView.ts +++ b/src/hooks/useScrollCursorIntoView.ts @@ -2,7 +2,6 @@ import { throttle } from 'lodash' import { useEffect, useRef } from 'react' import scrollCursorIntoView from '../device/scrollCursorIntoView' import testFlags from '../e2e/testFlags' -import isAllSelected from '../selectors/isAllSelected' import store from '../stores/app' import editingValueStore from '../stores/editingValue' @@ -11,13 +10,13 @@ const throttledScrollCursorIntoView = throttle((y: number, height: number) => sc // Expose the throttled function so that tests can cancel its pending trailing call before asserting on the scroll position. testFlags.throttledScrollCursorIntoView = throttledScrollCursorIntoView -/** Returns true if every thought at the cursor's level is selected (Select All). In that case the whole level is - * already "in focus", so autoscrolling the cursor when a multicursor formatting command rewrites its value would be a - * disruptive, pointless jump. See #3995 Issue H. */ -const isSelectAllActive = () => { - const state = store.getState() - return Object.keys(state.multicursors).length > 0 && isAllSelected(state) -} +/** Returns true if a multiselection is active. While thoughts are multiselected, the single cursor is a transient + * artifact of executeCommandWithMulticursor iterating over (and then restoring) the cursor across each selected + * thought. Following it with a cursor autoscroll causes a disruptive up-and-down jump, and on mobile Safari the + * restored cursor can be scrolled behind the virtual keyboard. The single intended scroll is performed by the + * multicursor formatting command's onComplete handler (see LayoutTree), so cursor-follow autoscroll must stand down. + * See #3995 Issue F/H. */ +const isMulticursorActive = () => Object.keys(store.getState().multicursors).length > 0 /** Call scrollCursorIntoView when the y position of its container changes, or when the editing value changes. */ const useScrollCursorIntoView = (y: number, height: number) => { @@ -38,15 +37,17 @@ const useScrollCursorIntoView = (y: number, height: number) => { * React's render cycle runs and processes the effect above. That's why setTimeout is necessary here (#3083). */ setTimeout(() => { - // Do not autoscroll while a Select All multicursor format rewrites the cursor's value. See #3995 Issue H. - if (isSelectAllActive()) return + // Do not autoscroll to the transient cursor while a multicursor format rewrites the selected thoughts' values. + // The topmost selected thought is snapped into view once via the command's onComplete handler. See #3995 Issue F/H. + if (isMulticursorActive()) return throttledScrollCursorIntoView(sizeRef.current.y, sizeRef.current.height) }) }) useEffect(() => { - // Do not autoscroll while a Select All multicursor format reflows the cursor's position. See #3995 Issue H. - if (isSelectAllActive()) return + // Do not autoscroll to the transient cursor while a multicursor format reflows the selected thoughts' positions. + // The topmost selected thought is snapped into view once via the command's onComplete handler. See #3995 Issue F/H. + if (isMulticursorActive()) return scrollCursorIntoView(y, height) }, [height, y]) }