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

Selection box #4242

Open
wants to merge 11 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@softwareCobbler
Copy link

softwareCobbler commented Feb 3, 2019

This implements a "selectionBox" feature in Network's interaction module. With this option enabled, the user can press CTRL and left click and then drag a box around the canvas. Let go and it selects the nodes and/or edges within the box (there are option flags the user can set to have it select nodes / edges).
Open to feedback!
This was briefly discussed in #4233.
Thanks!
sbox

@softwareCobbler softwareCobbler force-pushed the softwareCobbler:selectionBox branch from f2ef455 to 4b272e5 Feb 3, 2019

@mojoaxel mojoaxel self-requested a review Feb 3, 2019

@softwareCobbler

This comment has been minimized.

Copy link
Author

softwareCobbler commented Feb 3, 2019

@mojoaxel mojoaxel requested a review from jgorene Feb 4, 2019

@jgorene

This comment has been minimized.

Copy link
Member

jgorene commented Feb 4, 2019

A priori, I didn't encounter any bugs at least after a few tests.
Subject to further testing, this seems ready to be implemented at the latest on v4.22.
Good work !

@softwareCobbler softwareCobbler force-pushed the softwareCobbler:selectionBox branch from 27e5fef to f2bec3e Feb 5, 2019

@softwareCobbler

This comment has been minimized.

Copy link
Author

softwareCobbler commented Feb 5, 2019

considering the line width option - since CanvasRenderingContext2D.lineWidth is in coordinate space, I'm not certain how to provide a sane default. The value the user provides will change in apparent thickness as the zoom scale changes. Recalculating upon camera scale changes is the obvious solution, but I don't know quite what units the user expects to see.
Should a value of 1.0 always be 1 screen pixel? Or 1% of the total canvas area?

@jgorene

This comment has been minimized.

Copy link
Member

jgorene commented Feb 5, 2019

@softwareCobbler Indeed and it's already very good like that, no problem 😌
I still have a lot to learn and it will take me a little more time to properly analyze the new option , anyway.

@mojoaxel Just by the way and nothing to do with this new interaction option but it seems that "zoomSpeed" option is obsolete... but maybe I missed something?

@softwareCobbler

This comment has been minimized.

Copy link
Author

softwareCobbler commented Feb 7, 2019

Got the lineWidth option up and running for this:

  • It is a value in screen pixels, the canvas zoom should not affect the pixel count.
  • lineWidth is kept greater than or equal to zero (negative values are forced to 0), and while the user can pass in real numbers, the passed-in values are rounded down to the nearest integer.
  • a zero value means we draw no border, and just fill in the appropriate fillRect
  • canvas bounds clamping has been adjusted to deal with different sized lineWidth's
@@ -541,6 +541,7 @@ ItemSet.prototype.show = function() {

/**
* Activates the popup timer to show the given popup after a fixed time.
* @param {any} popup

This comment has been minimized.

@mojoaxel

This comment has been minimized.

@softwareCobbler

softwareCobbler Feb 17, 2019

Author

cherry-picked this into #4256 to provide a more logical separation of concern (it's a bit out of place in this PR)

@mojoaxel mojoaxel self-assigned this Feb 8, 2019

@mojoaxel mojoaxel added this to the Minor Release v4.22 milestone Feb 8, 2019

Revert "fix failing test from ItemSet.js"
This reverts commit f2bec3e (ref
almende/vis PR #4256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment