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

Allow global keyboard shortcuts to work in framed editor #6705

Merged

Conversation

EvidentlyCube
Copy link
Contributor

I've been adding some keyboard navigation to my personal Wikis and I noticed one thing that annoyed the end of me - if I am in the middle of editing a tiddler I need to click away with the mouse for the shortcuts to work.

What changes:

  • Shortcuts defined with tag $:/tags/KeyboardShortcut will now work even if the focus is in the tiddler editor when using the toolbar (ie. framed editor is used).
  • And these shortcuts take priority over the styling shortcuts already defined in the editor.

What changes (technically):

The reason is that the framed editor uses an iframe, and thus the keyboard events don't reach the host's document in the first place. I've distilled the solution to two changes:

  • $tw.utils.addEventListeners() now supports registering the event to listen in capture phase.
  • framed.js adds the listener that listens to the global shortcuts defined with $:/tags/KeyboardShortcuts to the iframe document.

Thus what happens is that if define a global keyboard shortcut for, say, Ctrl+L, if you have focus on the framed editor the capture phase listener will detect the keypress, run the shortcut and stop propagation, preventing Create wikitext link from running. On the other hand if there is no global keyboard shortcut it'll happily continue.

Older solution:

At first I made two different changes:

  • framed.js added a listener to global shortcuts directly in this.domNode
  • keyboard.js replaced stopPropagation() with stopImmediatePropagation() to prevent the framed editor's shortcuts to also trigger

But this introduced a potential problem, where if you had a situation where these callbacks were used but you also wanted the underlying events to trigger, they wouldn't. Or if you had multiple listeners in the document.

Possible problems:

I don't see any, but also I am not that knowledgable with TW.

@Jermolene
Copy link
Owner

Thank you and apologies for the delay @EvidentlyCube.

@Jermolene Jermolene merged commit 8e64e21 into Jermolene:master Jun 11, 2022
@Jermolene
Copy link
Owner

Hi @EvidentlyCube on further testing I have concerns about the part of this PR that overrides global shortcuts to take priority over any locally defined shortcuts (with the keyboard widget). This is not backwards compatible and causes problems with existing apps.

In these cases we revert the entire PR and then make a new PR (the alternative approach of making a separate PR leads to a more complex commit history for developers to navigate). Would you be available to work on this? Many thanks.

Jermolene added a commit that referenced this pull request Jun 22, 2022
@EvidentlyCube
Copy link
Contributor Author

@Jermolene I'd be happy to improve this! I was worried this could potentially cause issues but I couldn't figure out a specific case.

I was thinking how to best achieve this functionality and I've got two ideas:

  1. A global configuration that determines if the global keyboard shortcuts take priority or not
  2. A new field that can be defined per-shortcut that determines if it takes priority over other things or not.

I personally like option 2 better - it's more flexible and I think it's a functionality niche enough there is no point in littering the options page more.

@saqimtiaz
Copy link
Contributor

@EvidentlyCube any optional ability for a global keyboard shortcut to take precedence over local shortcuts would not only have to work with the framed editor but also keyboard shortcuts assigned by the keyboard widget, which can wrap any content.

A use case to consider is a form with various data entry widgets and HTML wrapped with a keyboard widget that provides keyboard shortcuts for that form. The framed editor may be one component of such a form and the keyboard shortcut behaviour in it should not differ from other parts of the form that may have focus.

@EvidentlyCube
Copy link
Contributor Author

@saqimtiaz Thanks, I used Keyboard Widget before but didn't think about it. So that makes it two things to keep in mind - framed editor and keyboard widget. I'll do some experimenting to figure out if I can make it work.

@EvidentlyCube
Copy link
Contributor Author

Created a new PR: #6735

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.

None yet

3 participants