Fix/disable shortcuts in text inputs by Elimpizza · Pull Request #7510 · hypothesis/client

Elimpizza · 2026-02-24T18:37:23Z

Shortcut behavior fixed: Sidebar/Search shortcuts (/ and Ctrl/Cmd+K) now respect ignoreWhenEditable, so they do not fire while focus is in inputs/textareas/contenteditable elements (SearchIconButton.tsx). The manual guard for / remains; the global guard covers both.
Tests expanded: SearchIconButton tests updated to:
- Expect no shortcut activation in inputs for / and Ctrl/Cmd+K.
- Assert no action when the search panel is already open for both Ctrl+K (non-macOS) and Cmd+K (macOS).
- Add a direct handler test that the “/” shortcut returns early when invoked with an input target.
Helper unchanged: Shared shortcut infrastructure already supports ignoreWhenEditable; used here to enforce “no keybinds while typing.”

…nd tests covering inputs, contenteditable and non-editable cases

…WhenEditable

codecov · 2026-02-24T21:11:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.61%. Comparing base (b07c1b8) to head (2c752f9).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7510   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files         283      283           
  Lines       11877    11895   +18     
  Branches     2898     2905    +7     
=======================================
+ Hits        11831    11849   +18     
  Misses         46       46

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…arget.selectionStart

Copilot

Pull request overview

This PR updates the sidebar’s search-related keyboard shortcuts so they don’t trigger while the user is typing in editable elements, by leveraging a shared ignoreWhenEditable shortcut option and expanding test coverage to prevent regressions.

Changes:

Add ignoreWhenEditable support to installShortcut / useShortcut and cover it with new unit tests.
Update sidebar components to set ignoreWhenEditable: true for search/apply-updates shortcuts.
Expand SearchIconButton tests to ensure shortcuts don’t fire in inputs and don’t re-open an already-open panel.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/shared/shortcut.ts	Adds `ignoreWhenEditable` option and editable-target detection for shortcut suppression.
src/shared/test/shortcut-test.js	Adds tests verifying `ignoreWhenEditable` behavior.
src/sidebar/components/search/SearchIconButton.tsx	Enables `ignoreWhenEditable` for `/` and Ctrl/Cmd+K search shortcuts.
src/sidebar/components/search/SearchField.tsx	Enables `ignoreWhenEditable` for “open search” focus behavior.
src/sidebar/components/PendingUpdatesNotification.tsx	Prevents apply-updates shortcut from firing while typing.
src/sidebar/components/search/test/SearchIconButton-test.js	Updates and adds tests for the new shortcut suppression behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T16:35:30Z

src/shared/shortcut.ts

+  if (target instanceof HTMLTextAreaElement) {
+    return true;
+  }
+  if (target instanceof HTMLInputElement) {
+    try {
+      return target.selectionStart !== null;
+    } catch {
+      return false;
+    }
+  }


isEditableTarget is less comprehensive than the existing isEditableContext helper used elsewhere to avoid intercepting shortcuts in editable contexts (eg. it doesn’t account for role="textbox"|"searchbox"|"combobox", and it treats some editable <input> types as non-editable). This can cause ignoreWhenEditable shortcuts to still fire while users are typing in ARIA textboxes/comboboxes or certain input types, which undermines the goal of disabling shortcuts while typing (WCAG 2.1.4). Consider reusing isEditableContext (eg. moving it into a shared module) or matching its logic here, and add/update tests to cover role-based editable targets.

Suggested change

if (target instanceof HTMLTextAreaElement) {

return true;

}

if (target instanceof HTMLInputElement) {

try {

return target.selectionStart !== null;

} catch {

return false;

}

}

// Treat ARIA text input widgets as editable.

const role = target.getAttribute('role');

if (role === 'textbox' || role === 'searchbox' || role === 'combobox') {

return true;

}

if (target instanceof HTMLTextAreaElement) {

return true;

}

if (target instanceof HTMLInputElement) {

// Consider common text-like input types as editable. If no type is set,

// it defaults to "text" in HTML.

const type = (target.type || '').toLowerCase();

const editableInputTypes = [

'',

'text',

'search',

'email',

'url',

'tel',

'password',

];

return editableInputTypes.includes(type);

}

Copilot · 2026-02-26T16:35:30Z

src/shared/shortcut.ts

+
 /**
- * Install a shortcut key listener on the document.
+ *Install a shortcut key listener on the document.


JSDoc formatting regression: there is a missing space after the leading * in this comment (*Install...), which will render incorrectly in generated docs and makes the block inconsistent with surrounding JSDoc style.

Suggested change

*Install a shortcut key listener on the document.

* Install a shortcut key listener on the document.

Copilot · 2026-02-26T16:35:31Z

src/sidebar/components/search/SearchIconButton.tsx

  const modifierKey = isMacOS() ? 'meta' : 'ctrl';

-  useShortcut(shortcuts.openSearch, openSearch);
-  useShortcut(`${modifierKey}+k`, openSearch);
+  useShortcut(shortcuts.openSearch, openSearch, { ignoreWhenEditable: true });
+  useShortcut(`${modifierKey}+k`, openSearch, { ignoreWhenEditable: true });


With ignoreWhenEditable: true now enabled for both shortcuts, the inline comment and logic inside openSearch that describes allowing Ctrl/Cmd+K while focused in inputs is no longer accurate. Either update the comment to reflect the new behavior (no shortcuts while typing) and/or simplify the now-redundant input/textarea guard if it’s no longer needed.

src/sidebar/components/search/SearchField.tsx

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T16:25:40Z

src/shared/shortcut.ts

+function isEditableTarget(target: EventTarget | null): boolean {
+  if (!(target instanceof HTMLElement)) {
+    return false;
+  }
+  if (target.isContentEditable) {
+    return true;
+  }
+  if (target instanceof HTMLTextAreaElement) {
+    return true;
+  }
+  if (target instanceof HTMLInputElement) {
+    try {
+      return target.selectionStart !== null;
+    } catch {
+      return false;
+    }
+  }
+  return false;
+}


isEditableTarget does not treat elements with ARIA roles like role="textbox"/searchbox/combobox (or descendants) as editable. Elsewhere in the codebase, keyboard shortcut interception is disabled in these role-based editable contexts for WCAG 2.1.4. Consider extending isEditableTarget (or reusing/moving the existing isEditableContext helper) so ignoreWhenEditable also skips shortcuts while focus is inside role-based editable widgets (eg. custom editors).

Copilot · 2026-02-27T16:25:41Z

src/shared/test/shortcut-test.js

+      assert.notCalled(onPress);
+      document.body.removeChild(div);
+    });
+


If ignoreWhenEditable is intended to cover all editable contexts (including textarea and role-based editable widgets), the new tests only cover <input> and contenteditable. Consider adding coverage for <textarea> and an element with role="textbox" (and/or a descendant of one) so regressions in the editable-target detection don’t slip in.

Suggested change

it('does not trigger when target is a textarea if ignoreWhenEditable is set', () => {

const onPress = sinon.stub();

const textarea = document.createElement('textarea');

document.body.appendChild(textarea);

const removeShortcut = installShortcut('a', onPress, {

ignoreWhenEditable: true,

});

const event = new KeyboardEvent('keydown', { key: 'a', bubbles: true });

textarea.dispatchEvent(event);

removeShortcut();

assert.notCalled(onPress);

document.body.removeChild(textarea);

});

it('does not trigger when target has role="textbox" if ignoreWhenEditable is set', () => {

const onPress = sinon.stub();

const div = document.createElement('div');

div.setAttribute('role', 'textbox');

document.body.appendChild(div);

const removeShortcut = installShortcut('a', onPress, {

ignoreWhenEditable: true,

});

const event = new KeyboardEvent('keydown', { key: 'a', bubbles: true });

div.dispatchEvent(event);

removeShortcut();

assert.notCalled(onPress);

document.body.removeChild(div);

});

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T17:35:35Z

src/shared/shortcut.ts

@@ -70,10 +70,38 @@ export type ShortcutOptions = {
   * `document.body`.


ShortcutOptions.rootElement docs say the listener defaults to document.body, but installShortcut actually defaults to document.documentElement. Please update the doc comment to match the actual default to avoid misleading API consumers.

Suggested change

* `document.body`.

* `document.documentElement`.

Copilot · 2026-02-27T17:35:36Z

src/shared/shortcut.ts

+    target.closest('[role="textbox"], [role="searchbox"]') instanceof
+    HTMLElement


isEditableTarget doesn’t treat [role="combobox"] as editable, but elsewhere in the codebase isEditableContext includes it for shortcut suppression (WCAG 2.1.4). Add combobox to this role check (or reuse the existing isEditableContext helper) so shortcuts are also ignored while interacting with combobox widgets.

Suggested change

target.closest('[role="textbox"], [role="searchbox"]') instanceof

HTMLElement

target.closest(

'[role="textbox"], [role="searchbox"], [role="combobox"]',

) instanceof HTMLElement

Copilot · 2026-02-27T17:35:36Z

src/shared/shortcut.ts

+    try {
+      return target.selectionStart !== null;
+    } catch {
+      return false;
+    }


In isEditableTarget, the HTMLInputElement branch treats inputs as editable only if selectionStart !== null, and returns false if reading selectionStart throws. This means ignoreWhenEditable won’t suppress shortcuts for some editable input types (eg type=number where selectionStart can throw). Consider treating all <input> elements as editable (or at least treating the exception case as editable) to match the intended semantics of ignoreWhenEditable.

Suggested change

try {

return target.selectionStart !== null;

} catch {

return false;

}

return true;

Copilot · 2026-02-27T17:35:36Z

src/shared/shortcut.ts

+
 /**
- * Install a shortcut key listener on the document.
+ *Install a shortcut key listener on the document.


JSDoc formatting: there’s a missing space after the leading * in this comment (*Install...), which breaks standard JSDoc formatting and may affect generated docs.

Suggested change

*Install a shortcut key listener on the document.

* Install a shortcut key listener on the document.

Copilot · 2026-02-27T17:35:37Z

src/shared/test/shortcut-test.js

+    it('fires shortcut when input selectionStart cannot be read', () => {
+      const onPress = sinon.stub();
+      const removeShortcut = installShortcut('a', onPress, {
+        ignoreWhenEditable: true,
+      });
+
+      const input = document.createElement('input');
+      Object.defineProperty(input, 'selectionStart', {
+        get() {
+          throw new Error('no selectionStart');
+        },
+      });
+      const event = new KeyboardEvent('keydown', { key: 'a', bubbles: true });
+      Object.defineProperty(event, 'target', { value: input });
+
+      document.documentElement.dispatchEvent(event);
+      removeShortcut();
+
+      assert.called(onPress);
+    });


This test asserts that shortcuts still fire when selectionStart can’t be read, which contradicts the goal of ignoreWhenEditable (don’t fire while typing) and the existing isEditableContext behavior (all INPUT/TEXTAREA are editable). If isEditableTarget is updated to treat these inputs as editable (recommended), this test should be updated to expect the shortcut NOT to trigger.

Elimpizza added 4 commits February 24, 2026 13:55

fix(infra): added ignoreWhenEditable support, editable target guard a…

38304c9

…nd tests covering inputs, contenteditable and non-editable cases

fix(sidebar): opt sidebar/search/ apply updates shortcuts into ignore…

6684475

…WhenEditable

fix(testcase): corrected flow

d09f5dd

fix(test): added missing testcase to get to 100% coverage

2a6f997

fix(text input detection): better definition of 'editable' by using t…

534d6ac

…arget.selectionStart

Elimpizza marked this pull request as ready for review February 25, 2026 13:13

Elimpizza requested review from Copilot and karenrasmussen February 25, 2026 13:13

Copilot started reviewing on behalf of Elimpizza February 26, 2026 16:32 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Elimpizza and others added 2 commits February 26, 2026 14:03

Update src/sidebar/components/search/SearchField.tsx

77d2174

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix(coverage): 100%

c42cdea

Elimpizza requested a review from Copilot February 27, 2026 16:21

Copilot started reviewing on behalf of Elimpizza February 27, 2026 16:22 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

fix(text detection): added two aria containers

2c752f9

Elimpizza requested a review from Copilot February 27, 2026 17:32

Copilot started reviewing on behalf of Elimpizza February 27, 2026 17:33 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

-  if (target instanceof HTMLTextAreaElement) {
-    return true;
-  }
-  if (target instanceof HTMLInputElement) {
-    try {
-      return target.selectionStart !== null;
-    } catch {
-      return false;
-    }
-  }
+  // Treat ARIA text input widgets as editable.
+  const role = target.getAttribute('role');
+  if (role === 'textbox' || role === 'searchbox' || role === 'combobox') {
+    return true;
+  }
+  if (target instanceof HTMLTextAreaElement) {
+    return true;
+  }
+  if (target instanceof HTMLInputElement) {
+    // Consider common text-like input types as editable. If no type is set,
+    // it defaults to "text" in HTML.
+    const type = (target.type || '').toLowerCase();
+    const editableInputTypes = [
+      '',
+      'text',
+      'search',
+      'email',
+      'url',
+      'tel',
+      'password',
+    ];
+    return editableInputTypes.includes(type);
+  }

	*Install a shortcut key listener on the document.
	* Install a shortcut key listener on the document.

		@@ -70,10 +70,38 @@ export type ShortcutOptions = {
		* `document.body`.

		target.closest('[role="textbox"], [role="searchbox"]') instanceof
		HTMLElement

Conversation

Elimpizza commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Elimpizza commented Feb 24, 2026 •

edited

Loading

codecov bot commented Feb 24, 2026 •

edited

Loading