Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignoring Hotkeys not working #264

Open
wesbos opened this issue Dec 21, 2023 · 2 comments
Open

Ignoring Hotkeys not working #264

wesbos opened this issue Dec 21, 2023 · 2 comments

Comments

@wesbos
Copy link

wesbos commented Dec 21, 2023

Looking at the example you have here: #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
Copy link
Owner

vnphanquang commented Dec 22, 2023

@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:

  1. checking for target in each callback, or
  2. use a 'centralized' on:shortcut event handler and check for target there before doing anything else, just as you wrote in Typing the i key in any input causes the global shortcut to fire syntaxfm/website#1464 (comment)

For example

  1. turn your handlePlayPause into:

    function handlePlayPause(detail) {
      if (ignoredTags.include(detail.originalEvent.target.tagName)) return;
      // ...
    }
  2. 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
Copy link
Author

wesbos commented Jan 17, 2024

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

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

No branches or pull requests

2 participants