A helper to add key combinations to buttons

[voddan]

I 've found that connecting a key combinations to a button is unusually hard.

Here is a helper I use in my project:

fun Button.keyCombination(key: KeyCodeCombination) {
    sceneProperty().onChange { scene ->
        if(scene != null) {
            if(key !in scene.accelerators) {
                scene.accelerators.put(key, Runnable { fire() })
            }
        }
    }
}

May be it might be abstracted to add key buildings to any action, but for buttons it should be really easy. Also note that theoretically one button can have more than one key combination.

[edvin]

Do you feel that it is necessary to listen to scene changes and reapply the keyCodeCombination or should we simply assign to the current or the first scene? The way you have written it, I suspect it would fail if the button is already part of a Scene (ie. the call to keyCombination is run later, for example in a Platform.runLater callback).

[edvin]

I also wonder if the most common use case is to use mnemonics, or do you generally use more exotic combinations than modifier+single letter?

[voddan]

The thing is that inside the dsl a button does not have a scene yet, so this.getScene() returns null. That's why the workaround with the listener was needed.

[voddan]

I have no statistics on the usage of key combinations (the only thing I've ever used is F7), but I see no reason to make this function less general

[edvin]

I realize that the button doesn't have a scene inside a dsl, but there are two (better) solutions to that.

1. Wrap the scene call in Platform.runLater (could fail in exotic environments)
2. Use your approach, but disconnect the listener once it has fired (fail safe)

I suggest going with approach number two. What do you think?

[voddan]

The 2nd sounds very good to me! Much better than my if(key !in scene.accelerators). Actually, both ifs will be eliminated.

[edvin]

Do you want to create a PR for this or should I? :)

[voddan]

Let me try and make a PR till the end of the week. If I can't - add it yourself :)

[edvin]

Good plan :)

[edvin]

This seems to be more complicated than we initially thought. If you open multiple versions of the same fragment you will bind these keys multiple times. Also, if you use the new InternalWindow, the accelerator needs to go away when the window closes. I'm looking for solutions now.

[edvin]

I think I found a good solution now. Instead of relying on the scene directly, I hook onto the dock / undock calls of UIComponent. I also renamed it to accelerator:

    fun Button.accelerator(combo: KeyCombination) {
        var oldCombo: Runnable? = null
        var currentScene: Scene? = null
        whenDocked {
            scene?.accelerators?.apply {
                currentScene = scene
                oldCombo = get(combo)
                put(combo, Runnable { fire() })
            }
        }
        whenUndocked {
            currentScene?.accelerators?.apply {
                if (oldCombo != null) put(combo, oldCombo)
                else remove(combo)
            }
        }
    }

I have committed it now. Does this work for you?

[edvin]

Usage example:

            button("Save") {
                accelerator(KeyCodeCombination.valueOf("Alt+S"))
                setOnAction {
                    alert(CONFIRMATION, "Saved!", "You did it!")
                }
            }

[voddan]

I am glad to hear that you are working on it. I myself could not find any time :(

accelerator is a good naming since it matches the documentation, but it is not very discoverable. When I needed it, I searched in CC (code completion) for "key", then read the docs, then searched for "acceler". But maybe this is a minor concern.

[edvin]

I also like the parallell to scene.accelerators, so I think we'll keep it, especially since it is actually implemented as accelerators on the scene. I'll make sure to mention "key, code, combination" in the docs, so at least it will be possible to find there :) Do you have a chance to try it before I release 1.5.8?

[voddan]

Sorry, no testing from me :) With our set up it turned out to be a real pain to update libraries :( But I have a new small project coming, I'll try it then!

[edvin]

OK, I added a framework test for it, so it seems good. I'll close the issue now.

[edvin]

Oh, I will add one more option to it: It could take a String directly, and in that case it would wrap it KeyCombination.valueOf(string). Now you can write:

accelerator("Alt+S")

[voddan]

I strongly believe that it should utilise enum values of KeyCode

It may be possibly combined with an overloaded operator operator fun KeyCode.plus(other: KeyCode): KeyCodeCombination

[edvin]

It still supports the KeyCombination parameter, but it has an overload to support a String, that's all :) We can for sure add the KeyCode.plus function as well, that's a good idea.