Ignoring Hotkeys not working

wesbos commented 11 months ago

Looking at the example you have here: https://github.com/vnphanquang/svelte-put/pull/258

Our code looks like this:

    function onShortcut(event: CustomEvent<ShortcutEventDetail>) {
        const keyboardEvent = event.detail.originalEvent;
        keyboardEvent.preventDefault();
        if ((keyboardEvent.target as HTMLElement)?.tagName === 'INPUT') {
            console.log('input focused, shortcut ignored');
            return;
        }
    }

<svelte:window
    on:shortcut={onShortcut}
    use:shortcut={{ trigger: getHotkeyTrigger('showHideHotkeys', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('minimize', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('mute', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('seekBackward', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('seekForward', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('increasePlaybackRate', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('decreasePlaybackRate', hotkeys) }}
    use:shortcut={{ trigger: getHotkeyTrigger('playPause', hotkeys) }}
/>

that function runs, and logs the ignored message. but it does not stop the event from propagating up to the window.

see syntax.fm - open the search and type hi - the i will make the player toggle up/down

vnphanquang commented 11 months ago

@wesbos thank you for reporting. When shortcut is placed on window, it's essentially window.addEventListener('keydown', ...). So the event has already reached window by the time your callback executes, calling preventDefault or stopPropagation effectively does nothing useful (except cases when you want to prevent browser default, for example overriding ctrl/cmd+k) .

That newly added example snippet might have been misleading, my apologies! I'll adjust to clarify this.

In your use case I think the issue can be resolved by either:

checking for target in each callback, or
use a 'centralized' on:shortcut event handler and check for target there before doing anything else, just as you wrote in https://github.com/syntaxfm/website/issues/1464#issuecomment-1866678171

For example

turn your handlePlayPause into:

function handlePlayPause(detail) {
  if (ignoredTags.include(detail.originalEvent.target.tagName)) return;
  // ...
}

turn your setup into the following:

<script>
  // your src/lib/hotkeys/Hotkeys.svelte
  // ...

  const allTriggers = [
    // can reuse your hotkeys object here
    ...Object.entries(hotkeys).map(([id, hotkey]) => {
      // exclude callback here and use in onShortcut bellow
      const { callback, ...trigger } = hotkey.trigger;
      return {
        id,
        ...trigger,
      };
    }),
  ];

  function onShortcut(event) {
    const detail = event.detail;
    const { originalEvent, trigger } = detail;
    const ignoredTags = [`INPUT`, `TEXTAREA`, `SELECT`, `OPTION`, `BUTTON`];
    if (ignoredTags.includes(originalEvent.target.tagName)) return;
    hotkeys[trigger.id].trigger.callback();
  }
</script>

<svelte:window use:shortcut={{ trigger: allTriggers }} on:shortcut={onShortcut} />

I try to reuse your code here, which makes it not so elegant but I hope the idea gets through.

If you think the API is not ergonomic enough, I'd love to hear what public interface you have in mind.

ps thanks for using this I never thought something i write would be used by wesbos himself.

wesbos commented 10 months ago

thanks so much for the clarification here! Your approach is about what I was thinking so I apprecaite you showing me that! Will have it implemented soon

vnphanquang / svelte-put

Ignoring Hotkeys not working #264