Victory: Allow brush panning when mouse is dragged outside the container

[sebastianquek]

Feature Requests

Checklist

• [x] I've read through the Docs and Guides to make sure this functionality doesn't already exist

• [x] I've searched open issues to make sure I'm not opening a duplicate issue

Description

Dragging a brush and moving your cursor outside the container will stop the panning.

Is it possible to somehow override this behaviour?

This feature can be seen in both D3 and Dygraphs:
https://bl.ocks.org/mbostock/6232620
http://dygraphs.com/gallery/#g/range-selector

[boygirl]

@sebastianquek I will investigate whether this will be possible in Victory's event system.

[chrisbrainerd]

Not sure if anyone's worked on this, but I've got a branch where I've got the basics working. Still got some kinks to work out, but can definitively say it's possible. Will keep working and get a PR in probably next week.

[chrisbrainerd]

Also I'm figuring this should be configurable by a prop - I'm assuming default behavior should be how things are now (if the mouse leaves the brushing box, stop brushing), but anyone have any opinions on what the prop should be called?

allowMouseEscape or something? I'm pretty stumped.

[boygirl]

@chrisbrainerd thanks for working on this! I'm looking forward to seeing your branch. Depending on the implementation, I think this might make sense as the default behavior. Performance concerns would flip my opinion on that. As for the naming issue... Maybe limitActiveBrushArea?

[chrisbrainerd]

Yeah personally I'd prefer this as the default behavior but I'm always concerned with what would essentially be a breaking change UX-wise (though in my opinion still a definite improvement). Either way ¯_(ツ)_/¯

[boygirl]

Yes, understandable. I have a couple breaking changes planned for next week anyway, so depending on timing this could be added to the release. I don't mind the breaking change if I'm able to provide and document an easy migration path for folks, and this would definitely have one, with just a boolean prop to add to get back to the default.

[chrisbrainerd]

@boygirl been working on this and have one case I'm having trouble with, wondering if you might be able to help.

So it works perfectly well as long as the mouse is still inside the bounding rect svg, but once it escapes that I need to:

1. figure out the mouse coords (easy, in the event)
2. figure out the root svg's coordinates on the page (where I'm having an issue).
3. figure out where on the page that is in relation to the svg's page coordinates, act accordingly

Is there a way you know of inside of brush-helpers.js to access the root svg's getBoundingClientRect value? The issue I'm having is that the mouse event won't always happen on the root, sometimes the target of the event will be a path on the svg which doesn't have a getBoundingClientRect, and it feels like writing some recursive mess to dig up until we find the svg will be sloppy.

Thanks!

[boygirl]

@chrisbrainerd

We're currently using a handful of methods in victory-core. This should help: https://github.com/FormidableLabs/victory-core/blob/master/src/victory-util/selection.js

getParentSVG uses recursion to get to the parent svg of an event target, but it isn't too sloppy, and we can cache it once we have it.

We use a matrix transformation rather than getBoundingClientRect to translate the mouse position to svg coordinate space.

[chrisbrainerd]

Hm, so the issue I'm seeing is that getParentSVG (called from Selection.getSVGEventCoordinates) expects the event to be propagating from a descendant of an svg. To get this to work right, we need to listen for mouse events not just over the parent SVG and its descendents, but anywhere on the document, so the way I wired it up is that in onMouseLeave, it attaches the onMouseMove handler to the whole document, and attaches onMouseUp to the document as well to clean up after.

So the issue at this point is that getParentSVG isn't able to be used anymore and I don't have access to the mouse position in the SVG coordinate space anymore. Any way you can think of around that problem, either by a different method of listening for mouse events outside of the parent SVG or by getting the mouse coordinates in the SVG coordinate space?

[boygirl]

Hmm... It makes sense that the brush event would need to _start_ within a containing svg, so you could still get that reference using getParentSVG and then store it and continue using the reference for calculations after the mouse leaves. You would add an argument to Selection.getSVGEventCoordinates so that it works like this:

getSVGEventCoordinates(evt, svg) {
   svg = svg || this.getParentSVG(evt.target);
   ...
  }

If you're having a hard time dealing with changes across multiple victory repos, let me know and I can either 1) help you get set up with an npm link or lank workflow or 2) just quickly publish the above change as a patch

[boygirl]

@chrisbrainerd heads up, I just did a bunch of refactoring in victory-core and I touched the Selection util. I ended up making the changes we discussed last week.

[chrisbrainerd]

:+1: got it pulled in, thanks!

So more questions since this is continuing to be trickier than I expected. The best implementation I can come with involves attaching an event listener to the window, not just the event target or parent.

Here's a fiddle of the concept, which seems to be how a number of other libraries are functioning: https://jsfiddle.net/wjrmL47v/26/

The issue I'm having here is managing the event listeners. As far as I can tell, valid event handler targets are "data", "parent", and "labels", and so using the build in event system I can't attach the onMouseMove listener to window or document.documentElement or anything like that. It's possible to do that just manually using document.addEventListener('mousemove', onMouseMove); etc, but that feels gross to me to have this be its own special case working differently than all the other event handlers in the repo.

Do you have any insight into the best way to attach a listener to the window in the Victory framework?

[boygirl]

@chrisbrainerd the issue you're encountering is why Victory's brush container doesn't continue dragging when the mouse leaves the container.

One possible solution might be to export an event handler from Victory that the user would need to manually attach to their document.

You might need to make the link between that event handler and Victory's event system with Victory's externalEventMutations https://formidable.com/open-source/victory/guides/events/#external-event-mutations

If you can get your proof of concept working with externalEventMutations we can figure out how to generalize it and make it nicer to use.

[chrisbrainerd]

@boygirl
https://github.com/chrisbrainerd/victory/tree/free-the-mouse-take-2
So I've got one working that uses externalEventMutations, defaults to current behavior and can optionally add a prop dontLimitActiveBrushArea which will also require adding externalEventMutations and updateMutations as props as well.

Seems to be working, but curious your thoughts! https://github.com/chrisbrainerd/victory/tree/free-the-mouse-take-2

[boygirl]

@chrisbrainerd that's exciting! I'll carve out some time tomorrow to take a look and review :)

[boygirl]

@chrisbrainerd actually, would you mind converting your work to an actual PR on victory-chart?

[chrisbrainerd]

Can do - left one comment on the branch though that would need to be handled - not sure how you'd like to handle that

[boygirl]

:/ manually I suppose. Sorry about that

[boygirl]

Or we can just pick up the conversation in your PR. I think the only sticky part is around window.addEventListener which I really would like to avoid.

[chrisbrainerd]

Hm what about this approach - instead of hardcoding window we just do hostNode.addEventListener and add in hostNode as a prop. I think this way you could essentially control what the bounds of the listener are - so you could for instance bind the mouse to whatever container you wanted, with the weird edge case that binding it to the window actually binds it to outside of the window too
https://jsfiddle.net/wjrmL47v/48/

[boygirl]

Yeah, that seems like a possibility. I definitely need to play with your branch a bit more

[chrisbrainerd]

:+1: fwiw I'd highly recommend starting with the fiddle if you're not 100% groking how the handlers work - took me ages and really could only start to understand the edge cases by using a minimal example.

[boygirl]

@chrisbrainerd please open a WIP PR of your branch on victory-chart when you get a chance. That will be the best way for us to collaborate.

[chrisbrainerd]

https://github.com/FormidableLabs/victory-chart/pull/601 opened :+1:

Combined the two mouse handlers so it's just one function being called with different args, and the only issue I'm seeing right now is that there's some funkiness if you manage to click and drag outside of the original svg and mouseup real fast. My hunch is that it's the way the mutations are stacking since the mouseMove is throttled. It's set with trailing: false, but still it _feels_ like it's sending the mouseUp mutation but then a mouseMove one gets queued after which resets the isPanning / isSelecting back to true after the mouse up handler sets them to false.

[Hypnosphi]

@boygirl @chrisbrainerd what's the current status of this? Do you need any help?

[chrisbrainerd]

@Hypnosphi very much so if it's still an issue, was a req I needed for an old job and don't anymore. No idea how out of date the draft PR is either, sorry - but hope it steers whoever picks this up in the right direction!

[Hypnosphi]

window.addEventListener which I really would like to avoid.

@boygirl may I ask why?