Skip to content

Fix Ctrl+A selecting grid instead of comment text (#12193)#12283

Merged
qunabu merged 4 commits intodevelopfrom
cursor/b-d-aftergetrowheader-fc9d
Apr 14, 2026
Merged

Fix Ctrl+A selecting grid instead of comment text (#12193)#12283
qunabu merged 4 commits intodevelopfrom
cursor/b-d-aftergetrowheader-fc9d

Conversation

@sl01k
Copy link
Copy Markdown
Contributor

@sl01k sl01k commented Apr 7, 2026

Context

Fixes #12193. When a user opens a comment editor (via context menu or Ctrl+Alt+M), types text, repositions the cursor inside the text, and presses Ctrl+A (or Cmd+A on Mac), the entire grid is selected instead of the text within the comment textarea.

Root cause: The Comments plugin's #onAfterDocumentKeyDown handler calls stopImmediatePropagation(event) in the afterDocumentKeyDown hook, which fires after the shortcut system has already processed the event. The grid context's Ctrl+A shortcut (selectAllCells()) executes before the Comments plugin can block it.

Fix: Added a keydown event listener directly on the comment textarea element that calls event.stopPropagation() to prevent keyboard events from bubbling up to the grid's shortcut system (which listens on document.documentElement). Events matching shortcuts registered in the plugin:comments context are allowed through via the new ShortcutManager.hasEventShortcut() API.

Architecture: To avoid duplicating the recorder's internal key matching logic, MODIFIER_KEYS, isModifierKey, and getPressedModifierKeys were extracted from recorder.js into shortcuts/utils.js as shared exports. A new getEventKeyCombinations(event, platformCheck) utility builds key combination arrays from a KeyboardEvent (with the OS-aware isMacOS guard for control/meta unification), and hasEventShortcut(contextName, event) was added to ShortcutManager as the public API. The Comments plugin calls this single method instead of reimplementing any shortcut matching.

How has this been tested?

  • Added two new E2E tests to keyboardShortcuts.spec.js verifying that Ctrl+A does not trigger grid "select all" when the comment textarea is focused (both via keyboard shortcut and context menu).
  • Ran all 133 Comments plugin E2E specs -- 0 failures.
  • Ran all 31 ShortcutManager E2E specs -- 0 failures.
  • Ran ESLint -- 0 errors.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. Unable to select comment text using Ctrl+A #12193

Affected project(s):

  • handsontable
  • @handsontable/angular-wrapper
  • @handsontable/react-wrapper
  • @handsontable/vue3

Checklist:

Open in Web Open in Cursor 

Stop keyboard events from propagating to the grid shortcut system
while the comment textarea is visible and focused. This prevents
grid-level shortcuts (e.g. Ctrl+A select all cells) from firing
when the user intends to interact with the comment editor text.

Shortcuts registered in the plugin:comments context (Escape,
Ctrl+Enter, Tab) are allowed through.

Co-authored-by: Marek Martuszewski <sl01k@users.noreply.github.com>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 7, 2026

More templates

npm i https://pkg.pr.new/handsontable@12283
npm i https://pkg.pr.new/@handsontable/react-wrapper@12283
npm i https://pkg.pr.new/@handsontable/vue3@12283
npm i https://pkg.pr.new/@handsontable/angular-wrapper@12283

commit: 6d3aac5

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

⚡ Performance Results

Scenario Scripting Rendering Painting Total Δ
Cell Editing 462 ms 240 ms 49 ms 751 ms -3.2% 🔵
Filtering 12 ms 2 ms 0 ms 14 ms -8.4% 🔵
Scroll Down 119 ms 20 ms 22 ms 160 ms -12.0% 🟢
Scroll Left 2288 ms 2179 ms 597 ms 5064 ms -6.2% 🔵
Scroll Right 398 ms 335 ms 104 ms 837 ms +1.2% 🟡
Scroll Up 1906 ms 1778 ms 403 ms 4086 ms +1.2% 🟡
Sorting 6 ms 1 ms 0 ms 8 ms -12.5% 🟢

All scenarios within tolerance ✅

📊 Full interactive report →

@sl01k sl01k marked this pull request as ready for review April 7, 2026 17:16
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Hardcoded shortcut allowlist duplicates registered shortcuts knowledge
    • Replaced the hardcoded key allowlist with a dynamic check against registered plugin:comments shortcuts and added an E2E regression test for a newly registered shortcut.

Create PR

Or push these changes by commenting:

@cursor push befdd71603
Preview (befdd71603)
diff --git a/handsontable/src/plugins/comments/__tests__/keyboardShortcuts.spec.js b/handsontable/src/plugins/comments/__tests__/keyboardShortcuts.spec.js
--- a/handsontable/src/plugins/comments/__tests__/keyboardShortcuts.spec.js
+++ b/handsontable/src/plugins/comments/__tests__/keyboardShortcuts.spec.js
@@ -334,6 +334,45 @@
     });
   });
 
+  describe('custom comments context shortcut', () => {
+    it('should execute a shortcut registered in the plugin context while the editor is focused', async() => {
+      handsontable({
+        data: createSpreadsheetData(4, 4),
+        rowHeaders: true,
+        colHeaders: true,
+        comments: true,
+      });
+
+      await selectCell(1, 1);
+      await keyDownUp(['control', 'alt', 'm']);
+      await waitForNextAnimationFrames(1);
+
+      let wasTriggered = false;
+      const editor = getPlugin('comments').getEditorInputElement();
+      const pluginContext = getShortcutManager().getContext('plugin:comments');
+
+      pluginContext.addShortcut({
+        keys: [['Control/Meta', 'S']],
+        callback: () => {
+          wasTriggered = true;
+        },
+        group: 'test',
+      });
+
+      const keyEvent = new KeyboardEvent('keydown', {
+        key: 's',
+        ctrlKey: true,
+        bubbles: true,
+        cancelable: true,
+      });
+
+      editor.dispatchEvent(keyEvent);
+      await waitForNextAnimationFrames(1);
+
+      expect(wasTriggered).toBe(true);
+    });
+  });
+
   describe('"Cmd/Ctrl" + "Enter"', () => {
     it('should close the comment and save the value (comment opened by keyboard shortcut)', async() => {
       handsontable({

diff --git a/handsontable/src/plugins/comments/comments.js b/handsontable/src/plugins/comments/comments.js
--- a/handsontable/src/plugins/comments/comments.js
+++ b/handsontable/src/plugins/comments/comments.js
@@ -13,6 +13,7 @@
 import { CellRange } from '../../3rdparty/walkontable/src';
 import { BasePlugin } from '../base';
 import { throwWithCause } from '../../helpers/errors';
+import { normalizeEventKey } from '../../shortcuts/utils';
 import CommentEditor from './commentEditor';
 import DisplaySwitch from './displaySwitch';
 import { getEditorAnchorWidth } from './utils';
@@ -851,15 +852,51 @@
       return;
     }
 
-    const { key, ctrlKey, metaKey } = event;
+    if (!this.#isCommentsContextShortcut(event)) {
+      event.stopPropagation();
+    }
+  }
 
-    const isEscape = key === 'Escape';
-    const isCtrlEnter = (ctrlKey || metaKey) && key === 'Enter';
-    const isTab = key === 'Tab';
+  /**
+   * Checks if the keydown event has a matching shortcut in the comments context.
+   *
+   * @param {KeyboardEvent} event The keydown event from the comment textarea.
+   * @returns {boolean}
+   */
+  #isCommentsContextShortcut(event) {
+    const shortcutContext = this.hot.getShortcutManager().getContext(SHORTCUTS_CONTEXT_NAME);
 
-    if (!isEscape && !isCtrlEnter && !isTab) {
-      event.stopPropagation();
+    if (!shortcutContext || typeof event.key !== 'string') {
+      return false;
     }
+
+    const key = normalizeEventKey(event);
+    const modifiers = [];
+
+    if (event.altKey) {
+      modifiers.push('alt');
+    }
+    if (event.ctrlKey) {
+      modifiers.push('control');
+    }
+    if (event.metaKey) {
+      modifiers.push('meta');
+    }
+    if (event.shiftKey) {
+      modifiers.push('shift');
+    }
+
+    if (shortcutContext.hasShortcut([key, ...modifiers])) {
+      return true;
+    }
+
+    if (event.ctrlKey || event.metaKey) {
+      const modifiersWithoutCtrlMeta = modifiers.filter(modifier => modifier !== 'control' && modifier !== 'meta');
+
+      return shortcutContext.hasShortcut([key, ...modifiersWithoutCtrlMeta, 'control/meta']);
+    }
+
+    return false;
   }
 
   /**

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread handsontable/src/plugins/comments/comments.js
@adrianspdev adrianspdev self-assigned this Apr 10, 2026
@adrianspdev adrianspdev self-requested a review April 10, 2026 07:26
Instead of hardcoding Escape/Ctrl+Enter/Tab in #onEditorKeyDown,
query the plugin:comments shortcut context dynamically via
hasShortcut(). This eliminates duplicated knowledge between
registerShortcuts() and the keydown handler, so future shortcut
changes in the comments context are picked up automatically.

Co-authored-by: Adrian Dusinkiewicz <adrianspdev@users.noreply.github.com>
Comment thread handsontable/src/plugins/comments/comments.js Outdated
Expose getEventKeyCombinations in shortcuts/utils.js to centralize
the event-to-keys conversion (normalizeEventKey + modifier extraction
+ control/meta unification). Add hasEventShortcut(contextName, event)
to ShortcutManager as a public API method.

The Comments plugin now uses this API instead of reimplementing the
recorder's internal key matching logic. The normalizeEventKey import
is removed from the plugin.

Co-authored-by: Adrian Dusinkiewicz <adrianspdev@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9bf9317. Configure here.

Comment thread handsontable/src/shortcuts/utils.js
Comment thread handsontable/src/shortcuts/utils.js
Move MODIFIER_KEYS, isModifierKey, and getPressedModifierKeys from
recorder.js to utils.js as shared exports. The recorder now imports
them instead of defining its own copies.

Add isMacOS guard to getEventKeyCombinations: the unified
control/meta form is only generated when the OS-native modifier is
pressed (Meta on macOS, Control on other systems), matching the
recorder's existing behavior.

Co-authored-by: Adrian Dusinkiewicz <adrianspdev@users.noreply.github.com>
@adrianspdev adrianspdev requested review from budnix and removed request for adrianspdev April 10, 2026 09:24
@qunabu qunabu merged commit 7ca35d6 into develop Apr 14, 2026
34 of 35 checks passed
@qunabu qunabu deleted the cursor/b-d-aftergetrowheader-fc9d branch April 14, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to select comment text using Ctrl+A

5 participants