Skip to content

Lasso & rectangular selections #154

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.

Sign up for GitHub

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

Merged
merged 35 commits into from
Jan 6, 2016
Merged

Lasso & rectangular selections #154

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d6f983e
agignore dist
alexcjohnson Dec 27, 2015
e536a91
point in polygon routine, with tests
alexcjohnson Dec 27, 2015
5cad4ff
lasso and selectbox skeleton
alexcjohnson Dec 28, 2015
ca60e63
propagate dragbox cursor to coverSlip while dragging
alexcjohnson Dec 29, 2015
76926d7
fix modebar logic and tests for select dragmodes
alexcjohnson Dec 29, 2015
78d2867
change polygon and its tests to multi-exports form
alexcjohnson Dec 29, 2015
f0e6cfb
polygon filtering algorithm
alexcjohnson Dec 29, 2015
c0a94f7
polygon filtering test
alexcjohnson Dec 29, 2015
12b43c6
special case of polygon for rectangles
alexcjohnson Dec 30, 2015
07e5cef
selection on scatter points
alexcjohnson Dec 31, 2015
b1a1c24
clear hover when drag starts
alexcjohnson Dec 31, 2015
1958475
didn't end up using a separate lasso handler
alexcjohnson Dec 31, 2015
3c40c41
crosshair it is
alexcjohnson Jan 4, 2016
188a0db
inline rectFirstEdgeTest
alexcjohnson Jan 4, 2016
8d0afb5
Merge branch 'master' into lasso
alexcjohnson Jan 4, 2016
1c5f325
lasso like it's 2016 baby!
alexcjohnson Jan 4, 2016
d0203c8
scatter.selectPoints uses scatter.hasMarkers
alexcjohnson Jan 4, 2016
5be72de
:cow2:
alexcjohnson Jan 4, 2016
057e4ac
prep for adding selection bounds to event data
alexcjohnson Jan 5, 2016
556cb83
split out scatter.selectPoints to a new file
alexcjohnson Jan 5, 2016
7b07a25
support selecting scatter text
alexcjohnson Jan 5, 2016
2a970fc
fx constants file
alexcjohnson Jan 5, 2016
d7abd18
more fx constants
alexcjohnson Jan 6, 2016
7addcec
BENDPX into constants
alexcjohnson Jan 6, 2016
d02d612
horizontal and vertical select boxes
alexcjohnson Jan 6, 2016
698376e
off-by-one error in select outline
alexcjohnson Jan 6, 2016
89c1655
clear selection on zoom/pan
alexcjohnson Jan 6, 2016
d8ed7a7
selection range event data
alexcjohnson Jan 6, 2016
7d897a4
show select icons only when they apply
alexcjohnson Jan 6, 2016
96e01b6
update modebar tests for new select icon logic
alexcjohnson Jan 6, 2016
bc04a15
desciption attribute value delimiters
alexcjohnson Jan 6, 2016
710791d
fix nonlinear axes in select
alexcjohnson Jan 6, 2016
342f991
:cow2:
alexcjohnson Jan 6, 2016
b5c090f
MINSELECT bigger than MINDRAG
alexcjohnson Jan 6, 2016
1d7745f
add lasso and selectbox icon
etpinard Jan 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix modebar logic and tests for select dragmodes
  • Loading branch information
@alexcjohnson
alexcjohnson committed Dec 29, 2015
commit 76926d77307c4dba0a2bf87e535e74aa18efbda5
13 changes: 11 additions & 2 deletions src/components/modebar/manage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,19 @@ function getButtonGroups(fullLayout, buttonsToRemove, buttonsToAdd) {

var hasCartesian = fullLayout._hasCartesian,
hasGL2D = fullLayout._hasGL2D,
allAxesFixed = areAllAxesFixed(fullLayout);
allAxesFixed = areAllAxesFixed(fullLayout),
dragModeGroup = [];

if((hasCartesian || hasGL2D) && !allAxesFixed) {
dragModeGroup = ['zoom2d', 'pan2d'];
}
if(hasCartesian) {
dragModeGroup.push('select2d');
dragModeGroup.push('lasso2d');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson (cc @chriddyp @jackparmer )

Should we show the select-box and lasso by default on all cartesian graphs and leave it up to plotly.js users or widget devs to add them using config.modeBarButtons ?

It seems strange to add two buttons that are useless for all but one trace type + mode combination on all cartesian graphs.

Alternatively, we could make getButtonsGroups in the mode bar manager look into fullData for scatter with marker traces and only show the buttons when present.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selection does have a use even without events or linking, for highlighting data while discussing or presenting it. And just because it's cool to play with 😎 So I wouldn't leave it entirely to developers to enable.

Then re: diving into fullData, I guess it depends how fast you think we'll add other trace types. (not sure if I'll be able to do it myself, but now it's just a matter of adding selectPoints methods to other modules) It seems pretty clear what select means for heatmaps and contour maps, and (at least for 1D as @chriddyp suggests, and I'm sure we can agree on 2D as well) what it means for bars, histograms, and boxes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it depends how fast you think we'll add other trace types.

It doesn't look like it will be hard to add selectPoints for the other trace types, but I doubt that it will be a priority for the next few months.

I'd vote for a check on fullData.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fullData check: 7d897a4
Seems to update icons correctly when I restyle mode or type.

}
if(dragModeGroup.length) addGroup(dragModeGroup);

if((hasCartesian || hasGL2D) && !allAxesFixed) {
addGroup(['zoom2d', 'pan2d', 'select2d', 'lasso2d']);
addGroup(['zoomIn2d', 'zoomOut2d', 'autoScale2d', 'resetScale2d']);
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if statement could be moved up here, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpsievert that would change the order which the buttons are displayed

Expand Down
26 changes: 17 additions & 9 deletions test/jasmine/tests/modebar_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ describe('ModeBar', function() {
it('creates mode bar (cartesian version)', function() {
var buttons = getButtons([
['toImage', 'sendDataToCloud'],
['zoom2d', 'pan2d'],
['zoom2d', 'pan2d', 'select2d', 'lasso2d'],
['zoomIn2d', 'zoomOut2d', 'autoScale2d', 'resetScale2d'],
['hoverClosestCartesian', 'hoverCompareCartesian']
]);
Expand All @@ -175,13 +175,14 @@ describe('ModeBar', function() {

expect(modeBar.hasButtons(buttons)).toBe(true);
expect(countGroups(modeBar)).toEqual(5);
expect(countButtons(modeBar)).toEqual(11);
expect(countButtons(modeBar)).toEqual(13);
expect(countLogo(modeBar)).toEqual(1);
});

it('creates mode bar (cartesian fixed-axes version)', function() {
var buttons = getButtons([
['toImage', 'sendDataToCloud'],
['select2d', 'lasso2d'],
['hoverClosestCartesian', 'hoverCompareCartesian']
]);

Expand All @@ -192,8 +193,8 @@ describe('ModeBar', function() {
var modeBar = gd._fullLayout._modeBar;

expect(modeBar.hasButtons(buttons)).toBe(true);
expect(countGroups(modeBar)).toEqual(3);
expect(countButtons(modeBar)).toEqual(5);
expect(countGroups(modeBar)).toEqual(4);
expect(countButtons(modeBar)).toEqual(7);
expect(countLogo(modeBar)).toEqual(1);
});

Expand Down Expand Up @@ -339,25 +340,32 @@ describe('ModeBar', function() {
it('updates mode bar buttons if modeBarButtonsToRemove changes', function() {
var gd = setupGraphInfo();
manageModeBar(gd);
var initialButtonCount = countButtons(gd._fullLayout._modeBar);

gd._context.modeBarButtonsToRemove = ['toImage', 'sendDataToCloud'];
manageModeBar(gd);

expect(countButtons(gd._fullLayout._modeBar)).toEqual(9);
expect(countButtons(gd._fullLayout._modeBar))
.toEqual(initialButtonCount - 2);
});

it('updates mode bar buttons if modeBarButtonsToAdd changes', function() {
var gd = setupGraphInfo();
manageModeBar(gd);

var initialGroupCount = countGroups(gd._fullLayout._modeBar),
initialButtonCount = countButtons(gd._fullLayout._modeBar);

gd._context.modeBarButtonsToAdd = [{
name: 'some button',
click: noop
}];
manageModeBar(gd);

expect(countGroups(gd._fullLayout._modeBar)).toEqual(6);
expect(countButtons(gd._fullLayout._modeBar)).toEqual(12);
expect(countGroups(gd._fullLayout._modeBar))
.toEqual(initialGroupCount + 1);
expect(countButtons(gd._fullLayout._modeBar))
.toEqual(initialButtonCount + 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started trying to make these tests more robust against changes... but was only convenient to do for this change test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

});

it('sets up buttons with modeBarButtonsToAdd and modeBarButtonToRemove', function() {
Expand All @@ -374,7 +382,7 @@ describe('ModeBar', function() {

var modeBar = gd._fullLayout._modeBar;
expect(countGroups(modeBar)).toEqual(6);
expect(countButtons(modeBar)).toEqual(10);
expect(countButtons(modeBar)).toEqual(12);
});

it('sets up buttons with modeBarButtonsToAdd and modeBarButtonToRemove (2)', function() {
Expand All @@ -394,7 +402,7 @@ describe('ModeBar', function() {

var modeBar = gd._fullLayout._modeBar;
expect(countGroups(modeBar)).toEqual(7);
expect(countButtons(modeBar)).toEqual(12);
expect(countButtons(modeBar)).toEqual(14);
});

it('sets up buttons with fully custom modeBarButtons', function() {
Expand Down