Fix moving cursor outside brackets (#6283)

* Add data attribute to modals for testing

* Add several tests for moving cursor outside/within brackets

Update tests to be more reliable and DRYer

Rename action menu to autocomplete menu; fix test race condition

Rename 'action modal' to 'autocomplete menu'

* Check for being outside of brackets on every keyup

Remove dead code

Co-authored-by: Tienson Qin <[email protected]>

e2e-tests/editor.spec.ts

@@ -197,7 +197,7 @@ test('copy and paste block after editing new block #5962', async ({ page, block
   await page.keyboard.press('Enter')
   await page.waitForTimeout(100)
   await page.keyboard.press('Enter')
-  
+
   await page.waitForTimeout(100)
   // Create a new block with some text
   await page.keyboard.insertText("Typed block")
@@ -212,6 +212,197 @@ test('copy and paste block after editing new block #5962', async ({ page, block
   await expect(page.locator('text="Typed block"')).toHaveCount(1);
 })
 
+test('#6266 moving cursor outside of brackets should close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['((', 'block-search']]) {
+    // First, left arrow
+    await createRandomPage(page)
+
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await autocompleteMenu.expectVisible(modalName)
+
+    await page.keyboard.press('ArrowLeft')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+
+    // Then, right arrow
+    await createRandomPage(page)
+
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await autocompleteMenu.expectVisible(modalName)
+
+    await page.waitForTimeout(100)
+    // Move cursor outside of the space strictly between the double brackets
+    await page.keyboard.press('ArrowRight')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+  }
+})
+
+// Old logic would fail this because it didn't do the check if @search-timeout was set
+test('#6266 moving cursor outside of parens immediately after searching should still close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['((', 'block-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    // TODO: Maybe remove these "text " entries in tests that don't need them
+    await block.mustFill('')
+    await page.waitForTimeout(550)
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await page.waitForTimeout(100)
+    await page.keyboard.type("some block search text")
+    await autocompleteMenu.expectVisible(modalName)
+
+    // Move cursor outside of the space strictly between the double parens
+    await page.keyboard.press('ArrowRight')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+  }
+})
+
+test('pressing up and down should NOT close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['((', 'block-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await autocompleteMenu.expectVisible(modalName)
+    const cursorPos = await block.selectionStart()
+
+    await page.keyboard.press('ArrowUp')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+    await expect(await block.selectionStart()).toEqual(cursorPos)
+
+    await page.keyboard.press('ArrowDown')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+    await expect(await block.selectionStart()).toEqual(cursorPos)
+  }
+})
+
+test('moving cursor inside of brackets should NOT close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['((', 'block-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await page.waitForTimeout(100)
+    if (commandTrigger === '[[') {
+      await autocompleteMenu.expectVisible(modalName)
+    }
+
+    await page.keyboard.type("search")
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+
+    // Move cursor, still inside the brackets
+    await page.keyboard.press('ArrowLeft')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+  }
+})
+
+test('moving cursor inside of brackets when autocomplete menu is closed should NOT open autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  // Note: (( behaves differently and doesn't auto-trigger when typing in it after exiting the search prompt once
+  for (const [commandTrigger, modalName] of [['[[', 'page-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await autocompleteMenu.expectVisible(modalName)
+
+    await block.escapeEditing()
+    await autocompleteMenu.expectHidden(modalName)
+
+    // Move cursor left until it's inside the brackets; shouldn't open autocomplete menu
+    await page.locator('.block-content').click()
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+
+    await page.keyboard.press('ArrowLeft')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+
+    await page.keyboard.press('ArrowLeft')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectHidden(modalName)
+
+    // Type a letter, this should open the autocomplete menu
+    await page.keyboard.type('z')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+  }
+})
+
+test('selecting text inside of brackets should NOT close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['((', 'block-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+
+    await page.keyboard.type("some page search text")
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+
+    // Select some text within the brackets
+    await page.keyboard.press('Shift+ArrowLeft')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+  }
+})
+
+test('pressing backspace and remaining inside of brackets should NOT close autocomplete menu', async ({ page, block, autocompleteMenu }) => {
+  for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['((', 'block-search']]) {
+    await createRandomPage(page)
+
+    // Open the autocomplete menu
+    await block.mustFill('')
+    for (const char of commandTrigger) {
+      await page.keyboard.type(char)
+      await page.waitForTimeout(10) // Sometimes it doesn't trigger without this
+    }
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+
+    await page.keyboard.type("some page search text")
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+
+    // Delete one character inside the brackets
+    await page.keyboard.press('Backspace')
+    await page.waitForTimeout(100)
+    await autocompleteMenu.expectVisible(modalName)
+  }})
 test('press escape when autocomplete menu is open, should close autocomplete menu only #6270', async ({ page, block }) => {
   for (const [commandTrigger, modalName] of [['[[', 'page-search'], ['/', 'commands']]) {
     await createRandomPage(page)

e2e-tests/fixtures.ts

@@ -3,7 +3,7 @@ import * as path from 'path'
 import { test as base, expect, ConsoleMessage, Locator } from '@playwright/test';
 import { ElectronApplication, Page, BrowserContext, _electron as electron } from 'playwright'
 import { loadLocalGraph, openLeftSidebar, randomString } from './utils';
-import { LogseqFixtures } from './types';
+import { autocompleteMenu, LogseqFixtures } from './types';
 
 let electronApp: ElectronApplication
 let context: BrowserContext
@@ -217,6 +217,30 @@ export const test = base.extend<LogseqFixtures>({
     use(block)
   },
 
+  autocompleteMenu: async ({ }, use) => {
+    const autocompleteMenu: autocompleteMenu = {
+      expectVisible: async (modalName?: string) => {
+        const modal = page.locator(modalName ? `[data-modal-name="${modalName}"]` : `[data-modal-name]`)
+        if (await modal.isVisible()) {
+          await page.waitForTimeout(100)
+          await expect(modal).toBeVisible()
+        } else {
+          await modal.waitFor({ state: 'visible', timeout: 1000 })
+        }
+      },
+      expectHidden: async (modalName?: string) => {
+        const modal = page.locator(modalName ? `[data-modal-name="${modalName}"]` : `[data-modal-name]`)
+        if (!await modal.isVisible()) {
+          await page.waitForTimeout(100)
+          await expect(modal).not.toBeVisible()
+        } else {
+          await modal.waitFor({ state: 'hidden', timeout: 1000 })
+        }
+      }
+    }
+    await use(autocompleteMenu)
+  },
+
   context: async ({ }, use) => {
     await use(context);
   },

e2e-tests/types.ts

@@ -38,11 +38,18 @@ export interface Block {
   selectionEnd(): Promise<number>;
 }
 
+export interface autocompleteMenu {
+  // Expect or wait for autocomplete menu to be or become visible
+  expectVisible(modalName?: string): Promise<void>
+  // Expect or wait for autocomplete menu to be or become hidden
+  expectHidden(modalName?: string): Promise<void>
+}
+
 export interface LogseqFixtures {
   page: Page;
   block: Block;
+  autocompleteMenu: autocompleteMenu;
   context: BrowserContext;
   app: ElectronApplication;
   graphDir: string;
 }
-

e2e-tests/utils.ts

@@ -42,6 +42,8 @@ export async function createRandomPage(page: Page) {
   await page.fill('[placeholder="Search or create page"]', randomTitle)
   // Click text=/.*New page: "new page".*/
   await page.click('text=/.*New page: ".*/')
+  // Wait for h1 to be from our new page
+  await page.waitForSelector(`h1 >> text="${randomTitle}"`, { state: 'visible' })
   // wait for textarea of first block
   await page.waitForSelector('textarea >> nth=0', { state: 'visible' })
 

src/main/frontend/components/editor.cljs

@@ -424,11 +424,11 @@
    {:not-matched-handler (editor-handler/keydown-not-matched-handler format)}))
 
 (defn- set-up-key-up!
-  [state input input-id search-timeout]
+  [state input input-id]
   (mixins/on-key-up
    state
    {}
-   (editor-handler/keyup-handler state input input-id search-timeout)))
+   (editor-handler/keyup-handler state input input-id)))
 
 (def search-timeout (atom nil))
 
@@ -438,7 +438,7 @@
         input-id id
         input (gdom/getElement input-id)]
     (set-up-key-down! state format)
-    (set-up-key-up! state input input-id search-timeout)))
+    (set-up-key-up! state input input-id)))
 
 (def starts-with? clojure.string/starts-with?)
 

src/main/frontend/handler/editor.cljs

@@ -1761,34 +1761,22 @@
 
   (handle-command-input-close id))
 
-(defn get-search-q
-  []
-  (when-let [id (state/get-edit-input-id)]
-    (when-let [input (gdom/getElement id)]
-      (let [current-pos (cursor/pos input)
-            pos (state/get-editor-last-pos)
-            edit-content (or (state/sub [:editor/content id]) "")]
-        (or
-         @*selected-text
-         (gp-util/safe-subs edit-content pos current-pos))))))
-
 (defn close-autocomplete-if-outside
   [input]
   (when (and input
              (state/get-editor-action)
              (not (wrapped-by? input page-ref/left-brackets page-ref/right-brackets)))
-    (when (get-search-q)
-      (let [value (gobj/get input "value")
-            pos (state/get-editor-last-pos)
-            current-pos (cursor/pos input)
-            between (gp-util/safe-subs value (min pos current-pos) (max pos current-pos))]
-        (when (and between
-                   (or
-                    (string/includes? between "[")
-                    (string/includes? between "]")
-                    (string/includes? between "(")
-                    (string/includes? between ")")))
-          (state/clear-editor-action!))))))
+    (let [value (gobj/get input "value")
+          pos (state/get-editor-last-pos)
+          current-pos (cursor/pos input)
+          between (gp-util/safe-subs value (min pos current-pos) (max pos current-pos))]
+      (when (and between
+                 (or
+                  (string/includes? between "[")
+                  (string/includes? between "]")
+                  (string/includes? between "(")
+                  (string/includes? between ")")))
+        (state/clear-editor-action!)))))
 
 (defn resize-image!
   [block-id metadata full_text size]
@@ -2803,7 +2791,7 @@
         nil))))
 
 (defn ^:large-vars/cleanup-todo keyup-handler
-  [_state input input-id search-timeout]
+  [_state input input-id]
   (fn [e key-code]
     (when-not (util/event-is-composing? e)
       (let [current-pos (cursor/pos input)
@@ -2924,11 +2912,11 @@
                 (state/set-editor-action-data! {:pos (cursor/get-caret-pos input)})
                 (state/set-editor-show-block-commands!))
 
-              (nil? @search-timeout)
-              (close-autocomplete-if-outside input)
-
               :else
               nil)))
+        
+        (close-autocomplete-if-outside input)
+        
         (when-not (or (= k "Shift") is-processed?)
           (state/set-last-key-code! {:key-code key-code
                                      :code code