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

Clicks are hard to handle with multiple targets #23

Closed
ggazzi opened this issue Dec 6, 2016 · 3 comments
Closed

Clicks are hard to handle with multiple targets #23

ggazzi opened this issue Dec 6, 2016 · 3 comments
Assignees
Labels

Comments

@ggazzi
Copy link
Contributor

ggazzi commented Dec 6, 2016

When dealing with multiple targets, the onMouseUp event is triggered before the onClick event.

As far as I understood, the onMouseDownKeyed event is used to register the current target, which may then be used by onDragBy and onClick. The onMouseUp is then used to remove the current target. If onMouseUp is triggered before onClick, however, we can't tell who the target was.

Is this behaving as intended? Should I be using a different pattern to determine the target of the click event?

Looking at Internal.elm (line 70), it seems as if onClick should occur before onDragBy. Maybe the order of the list is being changed somewhere, or maybe Cmd.batch doesn't guarantee an order?

My use case for this was a graph editor. Nodes can be dragged around, and their labels could be edited when clicked.

@zaboco
Copy link
Owner

zaboco commented Dec 6, 2016

Yes, that is a valid use-case I didn't think about when supporting multiple targets. It seems that, indeed, Cmd.batch has no guarantee for the order, and I can't think of a way to send multiple cmds sequentially (using Task.sequence would result in a Cmd (List msg), which is not ok).

I guess that the solution would be to pass the key to onClick as well. Or maybe there is a more elegant solution, I'll think about it.

@zaboco zaboco self-assigned this Dec 6, 2016
@zaboco zaboco added the bug label Dec 6, 2016
@ggazzi
Copy link
Contributor Author

ggazzi commented Dec 7, 2016

Having the key with the onClick event would be very handy :)

It should be possible just by adding the key to the DraggingTentative and Dragging states. If you think this is the way to go, I could submit a pull request.

@zaboco zaboco mentioned this issue Dec 8, 2016
3 tasks
@zaboco
Copy link
Owner

zaboco commented Dec 15, 2016

Fixed in version 2.0.0 @ggazzi

@zaboco zaboco closed this as completed Dec 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

2 participants