search

kieler / klighd-vscode

Eclipse Public License 2.0
8 stars 6 forks source link

Not selectable text should still be able to execute its actions #90

Closed soerendomroes closed 1 year ago

soerendomroes commented 2 years ago

See https://github.com/kieler/vscode/issues/32

soerendomroes commented 2 years ago

In piccolo not selectable elements are still used evaluated for their actions. E.g. if one clicks on an KStyledText with actions that is not selectable the pickedNode in processEvent in KlighdActionEventHandler is still the KSytledText. In VS Code this is not the case but the text cannot be clicked since it is not selectable.

soerendomroes commented 2 years ago

@NiklasRentzCAU want else do we need to know to implement it the same way as in piccolo?

NiklasRentzCAU commented 2 years ago

Turns out the action execution has nothing to do with the selectability of elements at all, but the pickability. The pickability of the elements is turned on for any rendering that has an action in AbstractKGERenderingController#1208..1210. So I will re-implement the action execution check on the client on this basis - check for an action on the rendering and try to execute it if available, if there is no action pick through the element and continue searching for an action.

NiklasRentzCAU commented 2 years ago

The intended behavior of KLighD action execution as it is implemented with Piccolo is not possible to implement with the event handler of Sprotty and needs dedicated event handlers on the SVG elements representing KRenderings with attached KActions.

The wanted behavior would check the elements from the topmost visible SVG element for attached actions, and continue to forward the event handling to the first element seen from the front with an action, stopping there and executing if triggers match. As SVG elements drawn directly behind other SVG elements do not need to be their parent element (see for example the +/- text for region collapsing/expanding in front of but not contained in the triangle behind) a single event handler bubbling its way through its parent hierarchy does not suffice to get the same behavior. Using Sprotty we currently define a single event handler on the top level SVG (the currentTarget in events) that triggers and has that top level SVG visible, as well as the deepest SVG element that was clicked on (the target in events) with no reference to the SVG element stack as it is drawn on screen, only the parent hierarchy, which is not the same.

A possible solution could add event listeners to every child SVG element and forward clicks to a central action handler, making sure to only execute an action on the top most element with an attached action (if applicable).

A simpler workaround could fix the current action handling to traverse through their parents to find actions and execute them (as is done currently) and ignore the fact that overlayed sibling elements cannot bubble the events, requiring a simple change in the semantics to make the +/- buttons work again.

soerendomroes commented 2 years ago

I don't understand fully the implications of the two alternatives but it seems to be that alternative one would be the solution we would benefit from in the long run.

NiklasRentzCAU commented 1 year ago

I went for the simpler workaround, as adding event listeners for every SVG element representing a rendering still had the same issue that the action execution would not be triggered on SVG elements physically behind other elements, which are not their parents. To make this work with the +/- collapse/expand buttons in the semantics, I implemented a simple fix for that issue over on the semantics repo.