Clear the browser selection when Cmd/Ctrl + Click or Shift + Click starts a multiselection

docs/cursor-and-caret.md

@@ -209,7 +209,7 @@ It is rendered as a sibling *after* the `ContentEditable`, and always in the sam
 
 ### Multi edit mode
 
-[`isMultiEditing`](../src/selectors/isMultiEditing.ts) is true when a multiselection is actually being *edited*: the keyboard is open, the cursor is a multicursor member, and the caret is in the cursor thought. Only Clear Thought puts the app in this state (via `useEditMode`'s relaxed multicursor guard); an ordinary multiselection leaves the caret outside any editable, so the predicate consults the DOM selection to tell the two apart. The commands that start or extend an ordinary multiselection uphold that by clearing the browser selection — [`selectAll`](../src/commands/selectAll.ts) as soon as it selects the thoughts, `cursorUp`/`cursorDown` on the next animation frame. A caret left behind in the cursor thought would otherwise read as multi edit mode, and the selection that [Copy Cursor](../src/commands/copyCursor.ts) saves and restores around the clipboard write would re-focus the editable and render a faux caret on every selected thought.
+[`isMultiEditing`](../src/selectors/isMultiEditing.ts) is true when a multiselection is actually being *edited*: the keyboard is open, the cursor is a multicursor member, and the caret is in the cursor thought. Only Clear Thought puts the app in this state (via `useEditMode`'s relaxed multicursor guard); an ordinary multiselection leaves the caret outside any editable, so the predicate consults the DOM selection to tell the two apart. The commands that start or extend an ordinary multiselection uphold that by clearing the browser selection — [`selectAll`](../src/commands/selectAll.ts) as soon as it selects the thoughts, `cursorUp`/`cursorDown` on the next animation frame, and Cmd/Ctrl + Click and Shift + Click as soon as they toggle or extend the selection (see `handleMultiselect` in [`Thought`](../src/components/Thought.tsx)). A caret left behind in the cursor thought would otherwise read as multi edit mode, and the selection that surfaces such as the [Command Universe](../src/components/DesktopCommandUniverse.tsx) and [Copy Cursor](../src/commands/copyCursor.ts) save and restore would re-focus the editable and render a faux caret on every selected thought.
 
 The caret is checked against the cursor thought specifically, not merely against some thought. Shift + ArrowUp/ArrowDown moves the cursor onto the next thought and only takes the caret out of the thought the multiselect started from on the following animation frame, so for that frame the caret is in a thought that is not the cursor. Accepting any thought would read that as multi edit mode and make every command below defer to the browser — most visibly Escape, which would exit an editing session that was never entered and leave the multiselection standing.
 

src/components/Thought.tsx

@@ -17,6 +17,7 @@ import { toggleMulticursorActionCreator as toggleMulticursor } from '../actions/
 import { isTouch } from '../browser'
 import { AlertType, REGEX_TAGS } from '../constants'
 import { MIN_CONTENT_WIDTH_EM } from '../constants'
+import * as selection from '../device/selection'
 import testFlags from '../e2e/testFlags'
 import useDragAndDropThought from '../hooks/useDragAndDropThought'
 import useDragHold from '../hooks/useDragHold'
@@ -32,6 +33,7 @@ import getChildren, { getAllChildrenAsThoughts, getChildrenRanked } from '../sel
 import getStyle from '../selectors/getStyle'
 import getThoughtById from '../selectors/getThoughtById'
 import isContextViewActive from '../selectors/isContextViewActive'
+import isMultiEditing from '../selectors/isMultiEditing'
 import rootedParentOf from '../selectors/rootedParentOf'
 import col1MaxWidthStore from '../stores/col1MaxWidthStore'
 import distractionFreeTypingStore from '../stores/distractionFreeTyping'
@@ -523,18 +525,25 @@ const ThoughtContainer = ({
 
       const mouseEvent = e as React.MouseEvent
 
-      // Shift + Click selects all thoughts between the clicked thought and the previously selected thought.
-      if (mouseEvent.shiftKey) {
-        e.preventDefault()
-        dispatch(selectBetween({ path }))
-        return
-      }
-
-      // Cmd/Ctrl + Click toggles the clicked thought in the multicursor selection.
-      if (isCommandKey(mouseEvent)) {
-        e.preventDefault()
-        dispatch(toggleMulticursor({ path }))
-      }
+      if (!mouseEvent.shiftKey && !isCommandKey(mouseEvent)) return
+
+      e.preventDefault()
+
+      dispatch((dispatch, getState) => {
+        // An ordinary multiselection leaves the caret outside any editable, which is how isMultiEditing tells it apart
+        // from a multiselection that is being edited (Clear Thought), as Select All does when it selects the thoughts.
+        // A caret left behind in the clicked thought would otherwise be restored by any surface that saves and restores
+        // the selection — the Command Universe on close, or Copy Cursor around the clipboard write — re-focusing the
+        // editable and rendering a faux caret on every selected thought (#5405).
+        // Checked before the dispatch below, since afterwards the clicked thought is a multicursor and the caret still
+        // in it would itself read as multi edit mode. Not while the multiselection is being edited, since clearing the
+        // caret would blur the thought being edited and exit the cleared state (see onBlur in Editable).
+        if (!isMultiEditing(getState())) selection.clear()
+
+        // Shift + Click selects all thoughts between the clicked thought and the previously selected thought, while
+        // Cmd/Ctrl + Click toggles the clicked thought in the multicursor selection.
+        dispatch(mouseEvent.shiftKey ? selectBetween({ path }) : toggleMulticursor({ path }))
+      })
     },
     [dispatch, path],
   )

src/e2e/puppeteer/__tests__/multiselect.ts

@@ -1,5 +1,6 @@
 import { KnownDevices } from 'puppeteer'
 import { HOME_DISPLAY_VALUE } from '../../../constants'
+import acknowledgeAiDisclosure from '../helpers/acknowledgeAiDisclosure'
 import click from '../helpers/click'
 import clickBullet from '../helpers/clickBullet'
 import clickThought from '../helpers/clickThought'
@@ -424,6 +425,32 @@
     expect(await textCursors()).toEqual(['auto', 'auto'])
   })
 
+  // https://github.com/cybersemics/em/issues/5405
+  it('does not render a faux caret when Define Term is run on a multiselection from the Command Universe', async () => {
+    await paste(`
+        - Novel
+        - Dictionary
+        - Notebook
+        `)
+
+    await acknowledgeAiDisclosure()
+
+    // the click leaves the real caret in Novel, which the multiselect then includes
+    await clickThought('Novel')
+    await multiselectThoughts(['Novel', 'Dictionary'])
+    await waitForHighlightedBullets(2)
+
+    await command('Define Term', { inputType: 'commandPalette' })
+
+    // the Command Universe restores the browser selection it saved when it opened, so the faux carets would be
+    // rendered on the frame after it unmounts
+    await page.waitForSelector('[data-testid=desktop-command-universe]', { hidden: true })
+    await nextFrame()
+    await nextFrame()
+
+    expect(await page.$$('[data-testid="faux-caret-multicursor"]')).toHaveLength(0)
+  })
+
   it('should delete all selected thoughts when Backspace is pressed with Select All active', async () => {
     await paste(`
         - A

src/e2e/puppeteer/helpers/acknowledgeAiDisclosure.ts

@@ -0,0 +1,10 @@
+import { page } from '../session'
+
+/** Acknowledges the AI data disclosure on the device, as a user who has already opted in to AI functionality has.
+ * Without it, the first AI command shows the disclosure modal instead of executing (see aiDisclosure.ts). */
+const acknowledgeAiDisclosure = () =>
+  page.evaluate(() => {
+    localStorage.setItem('aiDisclosureAcknowledged/v1', '1')
+  })
+
+export default acknowledgeAiDisclosure