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

Make it possible to change keyboard binding priorities #2423

Closed
alex6480 opened this issue Dec 8, 2018 · 5 comments
Closed

Make it possible to change keyboard binding priorities #2423

alex6480 opened this issue Dec 8, 2018 · 5 comments

Comments

@alex6480
Copy link

alex6480 commented Dec 8, 2018

Currently when you use the documented addBinding function there are some key handlers that are impossible to implement. Take the following example.

I want to implement a handler that makes it so that shift + enter on a list item creates an indent instead of another list item (similar to OneNote).

this.quill.keyboard.addBinding({
    key: 13,
    collapsed: true,
    format: ["list"],
    shiftKey: true,
}, (x: any, y: any) => {
    alert("TEST");
});

However, this handler never runs. By digging into the code I found out that the reason is that my handler is inserted with a lower priority than the default handlers.

image

My handler is at the end of the list and the handler above is less specific.

If I manually insert my handler at the top of the list it works fine.

this.quill.keyboard.bindings[13].unshift({
    key: 13,
    collapsed: true,
    format: ["list"],
    shiftKey: true,
    handler: (x: any, y: any) => {
        alert("TEST");
    },
});

But this is undocumented and may break in the future.
I think that you should either sort handlers by specificity (so more specific handlers are inserted before less specific handlers) or add an API to insert a handler at a specific position.

@douweknook
Copy link

douweknook commented Jan 9, 2019

I ran into a similar problem but found it is possible to prioritize your own handlers by using the configuration rather than addBinding. Docs: https://quilljs.com/docs/modules/keyboard/#configuration

Some bindings are essential to preventing dangerous browser defaults, such as the enter and backspace keys. You cannot remove these bindings to revert to native browser behaviors. However since bindings specified in the configuration will run before Quill’s defaults, you can handle special cases and propagate to Quill’s otherwise.

Adding a binding with quill.keyboard.addBinding will not run before Quill’s because the defaults bindings will have been added by that point.

@georgehb
Copy link

I ran into a similar problem but found it is possible to prioritize your own handlers by using the configuration rather than addBinding. Docs: https://quilljs.com/docs/modules/keyboard/#configuration

Have you actually gotten this to work? I've found that I can overwrite a default binding if I give it the same name (although it actually merges the bindings, so additional options like prefix and modifier keys get set unintentionally if they were present in the original).

But I can't get my binding to run before a default as the documentation implies you can. Looking through the source code I can see the problem. The ordering the bindings get applied relies on the ordering of Object.keys, which puts them in the order they got added to the object.

modules/keyboard.js:
2020-02-17-182505

As the user configs are added after the defaults as overrides using extend they get added last.

core/quill.js:
2020-02-17-182351

The workaround I've found is naming the binding an integer or an integer string ie "5" as Object.keys orders these keys first regardless of insertion order. But this solution feels like a bit of a hack and could stop working in the future.

Really I think there should be flag on the binding that tells quill to add it first. And when you overwrite an existing binding, ideally it should just use the new binding and not merge them. It should also probably give a warning when you do this, similar to when you replace an existing format or module.

@tony
Copy link

tony commented Jun 18, 2020

I got bit by this in another angle - between this and #1940 (comment).

If someone in this issue (or other issues) wants to do a refactor of modules/keyboard.js to address this, it'd be nice. I think it's more instrumentation than a bug.

There are many cases where having better API access to keyboard events (adding, removing, switching/updating handlers) would make sense after initialization. Also another one would be avoiding adding keybindings in the constructor, or pulling them out of the class and making them available in the exports to tweak.

In my case I was updating enter, which apparently can't be done via addBinding, since some bindings are in the constructor of the module itself: https://github.com/quilljs/quill/blob/develop/modules/keyboard.js#L25.

@Jockeyvb
Copy link

Jockeyvb commented Jul 21, 2023

` var keys = [{ key: 13 }, { key: 38 }, { key: 40 }, { key: 27 }, { key: 32 }];
var oldbindings = quill.keyboard.bindings["13"];
quill.keyboard.bindings["13"] = [];

        $.each(keys, function () {
            quill.keyboard.addBinding(this, HookKey);//绑定特殊使用键
        })
        $.each(oldbindings, function () {
            quill.keyboard.bindings["13"].push(this);
        })`

is ok ,set enter key first item

using jquery

@quill-bot
Copy link

Quill 2.0 has been released (announcement post) with many changes and fixes. If this is still an issue please create a new issue after reviewing our updated Contributing guide 🙏

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

6 participants