Keep Command Center open during multicursor command execution

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, string> = {
+  [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)
   }

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)
+    })
+  })
+})

src/commands/bold.ts

@@ -1,7 +1,9 @@
 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 { 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. */
@@ -10,14 +12,14 @@ 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 => {
     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)
 }

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 => {

src/commands/italic.ts

@@ -1,7 +1,9 @@
 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 { 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. */
@@ -12,12 +14,12 @@ 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))
   },
   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)
 }

src/commands/strikethrough.ts

@@ -1,7 +1,9 @@
 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 { 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. */
@@ -12,13 +14,13 @@ 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))
   },
   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)
 }

src/commands/underline.ts

@@ -1,7 +1,9 @@
 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 { 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. */
@@ -14,12 +16,13 @@ const underline: Command = {
   keyboard: { key: 'u', meta: true },
   multicursor: {
     preventSetCursor: true,
+    onComplete: scrollMulticursorIntoViewOnComplete,
   },
   canExecute: state => {
     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)
 }

src/components/LayoutTree.tsx

@@ -7,14 +7,19 @@ 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 isAllSelected from '../selectors/isAllSelected'
+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 +252,23 @@ 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()
+    // 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
+    // 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.)