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

[Issue-2005] Don't prevent left arrow events on Break leafs #2929

Closed

Conversation

MikaelCarpenter
Copy link

Links

Why

I'm currently trying to add a custom key binding for the left arrow. It works except for on empty lines (<p><br /></p>)

Description of the problem

  • Keyboard has a default keybinding 'embed left' that is called before my custom key binding
  • Currently 'embed left' is returning false when the cursor is in an instance of an EmbedBlot (code)
  • My custom key binding is prevented based on the matches.some(...) logic (code)

What

  • I updated the 'embed left' handler to ignore Break like it does for all other non EmbedBlots

Alternative Solutions

  • Make the custom key bindings fire before the default ones
    • It seems as though custom key bindings are meant to fire before the default key bindings, if you pass them in as part of the config. However that doesn't seem to be the case anymore (see Codesandbox for how binding is added). So maybe the fix should be fore that instead?
  • Alter the Break class to not inherit from EmbedBlot
    • I looked into this briefly and it seems like a much more complex change

Caveats

  • I don't have a test for this, but I can add them once we can agree on a solution we want
    • i tested manually that my use case works as well as the use case that the original commit I referenced was trying to fix (using arrows to navigate past embeds like formulas)

@luin luin self-requested a review December 14, 2022 14:12
@luin luin added this to the 2.0 milestone Dec 14, 2022
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.

2 participants