simplify ignoreWhenEditable guard for shortcuts

Author: Elimpizza

src/shared/shortcut.ts

@@ -75,29 +75,10 @@ export type ShortcutOptions = {
 };
 
 function isEditableTarget(target: EventTarget | null): boolean {
-  if (!(target instanceof HTMLElement)) {
-    return false;
-  }
-  if (target.isContentEditable) {
-    return true;
-  }
-  if (
-    target.closest('[role="textbox"], [role="searchbox"]') instanceof
-    HTMLElement
-  ) {
-    return true;
-  }
-  if (target instanceof HTMLTextAreaElement) {
-    return true;
-  }
-  if (target instanceof HTMLInputElement) {
-    try {
-      return target.selectionStart !== null;
-    } catch {
-      return false;
-    }
-  }
-  return false;
+  return (
+    target instanceof HTMLElement &&
+    (target.isContentEditable || ['INPUT', 'TEXTAREA'].includes(target.tagName))
+  );
 }
 
 /**

src/shared/test/shortcut-test.js

@@ -202,67 +202,6 @@ describe('shared/shortcut', () => {
 
       assert.called(onPress);
     });
-
-    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);
-    });
-
-    it('does not trigger when target has role="textbox" if ignoreWhenEditable is set', () => {
-      const onPress = sinon.stub();
-      const textbox = document.createElement('div');
-      textbox.setAttribute('role', 'textbox');
-      document.body.appendChild(textbox);
-
-      const removeShortcut = installShortcut('a', onPress, {
-        ignoreWhenEditable: true,
-      });
-
-      const event = new KeyboardEvent('keydown', { key: 'a', bubbles: true });
-      Object.defineProperty(event, 'target', { value: textbox });
-      document.documentElement.dispatchEvent(event);
-
-      removeShortcut();
-      document.body.removeChild(textbox);
-
-      assert.notCalled(onPress);
-    });
-
-    it('still triggers when target has a non-editable ARIA role if ignoreWhenEditable is set', () => {
-      const onPress = sinon.stub();
-      const nonEditable = document.createElement('div');
-      nonEditable.setAttribute('role', 'button');
-      document.body.appendChild(nonEditable);
-
-      const removeShortcut = installShortcut('a', onPress, {
-        ignoreWhenEditable: true,
-      });
-
-      const event = new KeyboardEvent('keydown', { key: 'a', bubbles: true });
-      Object.defineProperty(event, 'target', { value: nonEditable });
-      document.documentElement.dispatchEvent(event);
-
-      removeShortcut();
-      document.body.removeChild(nonEditable);
-
-      assert.called(onPress);
-    });
   });
 
   describe('useShortcut', () => {

src/sidebar/components/search/SearchIconButton.tsx

@@ -22,16 +22,6 @@ function useSearchKeyboardShortcuts(store: SidebarStore) {
 
   const openSearch = useCallback(
     (event: KeyboardEvent) => {
-      // When user is in an input field, respond to CMD-/CTRL-K keypresses,
-      // but ignore '/' keypresses
-      if (
-        !event.metaKey &&
-        !event.ctrlKey &&
-        event.target instanceof HTMLElement &&
-        ['INPUT', 'TEXTAREA'].includes(event.target.tagName)
-      ) {
-        return;
-      }
       prevFocusRef.current = document.activeElement as HTMLOrSVGElement | null;
       if (!store.isSidebarPanelOpen('searchAnnotations')) {
         store.openSidebarPanel('searchAnnotations');

src/sidebar/components/search/test/SearchIconButton-test.js

@@ -155,40 +155,6 @@ describe('SearchIconButton', () => {
       assert.notCalled(fakeStore.openSidebarPanel);
     });
 
-    it('returns early when "/" shortcut handler is invoked while focus is in an input', () => {
-      const handlers = [];
-
-      $imports.$mock({
-        '../../../shared/shortcut': {
-          useShortcut: (shortcut, handler) =>
-            handlers.push({ shortcut, handler }),
-        },
-      });
-
-      const input = document.createElement('input');
-      document.body.append(input);
-
-      try {
-        createSearchIconButton();
-
-        const openSearchHandler = handlers.find(
-          h => h.shortcut === '/',
-        )?.handler;
-        assert.ok(openSearchHandler, 'handler for "/" should be registered');
-
-        openSearchHandler({
-          key: '/',
-          metaKey: false,
-          ctrlKey: false,
-          target: input,
-        });
-
-        assert.notCalled(fakeStore.openSidebarPanel);
-      } finally {
-        input.remove();
-      }
-    });
-
     it('opens search panel for macOS when "Cmd-K" is pressed outside of the component element', () => {
       fakeIsMacOS.returns(true);
       createSearchIconButton();