scijava / ui-behaviour

Configurable input handling, via mapped behaviours
Other
4 stars 2 forks source link

VisualEditorPanel: finer grain listeners #20

Closed xulman closed 4 years ago

xulman commented 4 years ago

Hi, I propose to add another layer of listeners into the VisualEditorPanel, and to rename to original ones (without breaking public API).

The original ones (renamed now into itemChangedListeners) were triggered everytime an item was changed in the GUI and were not triggered when the "Apply" button was pressed. The user-land code was therefore updated incrementally about a presence of change (no details about the change per se were provided via the listener notification 1) and had to rescan somehow the GUI to find the change and update its own data. The user-land code never learnt that "apply" was pressed, so how should the code learn what has been changed and when to take these changes seriously?

1: Would be nice if the listeners would hear what triggered them -- what has been just changed. but we either break the API (by introducing param to the listeners) or create yet another type of listeners

The added listeners layer (configChangedListeners) is triggered only when "Apply" button is used, so they know that now is the right time to process the underlying InputTriggerConfig to readout the fresh new wanted state of key bindings... (or I just totally misunderstood the whole thing......)

Cheers, Vlado

xulman commented 4 years ago

The last commit f99fb9f12b22e4bc9a2d507b882f0c29533d0b7f was originally intended to become a PR on its own, but it is closely related to the former two commits.

The envisioned (example) usage is as follows, here in Kotlin:

        //behaviourMap, config and (the associated) inputMap was defined earlier

        val cdb = CommandDescriptionBuilder()
        behaviourMap.keys().forEach { b -> cdb.addCommand(b,"all","") }
        //NB: don't know how to read out all behaviours' string IDs from config

        val editorPanel = VisualEditorPanel(config, cdb.get())

        val frame = JFrame(editorTitle)
        frame.contentPane.add(editorPanel)
        frame.pack()
        frame.isVisible = true

        editorPanel.addConfigCommittedListener { config.resetThisMapFor( inputMap, "all" ) }
        //with every "apply button" the config gets updated (as part of the functionality of the Editor)
        //with every "apply button", the associated inputMap gets updated too
tpietzsch commented 4 years ago

@xulman see inline comment about commit f99fb9f

The other changes regarding the VisualEditorPanel listeners are fine.

xulman commented 4 years ago

please drop the commit https://github.com/scijava/ui-behaviour/pull/20/commits/f99fb9f12b22e4bc9a2d507b882f0c29533d0b7f of mine from this PR I confirm, it was easy to use the updateKeyConfig mechanism: scenerygraphics/scenery@ed025a7

tpietzsch commented 4 years ago

Ok, I dropped it. I also revised the listeners stuff to use scijava-listeners. So

editorPanel.addConfigChangeListener(listener)

becomes

editorPanel.configChangeListeners().add(listener)

And

editorPanel.addConfigCommittedListener(listener)

becomes

editorPanel.configCommittedListeners().add(listener)