Set enabled:false when filter target array is empty by etpinard · Pull Request #3766 · plotly/plotly.js

etpinard · 2019-04-12T16:05:33Z

closes#2908 - by implementing #2908 (comment)

cc @plotly/plotly_js

before/after example: https://codepen.io/etpinard/pen/wYdppB / https://codepen.io/etpinard/pen/WWOVBV

- see #2908 for more info, - this fixes many potential problems downstream

archmoj · 2019-04-12T16:23:50Z

 if(enabled) {
+ var target = coerce('target');
+
+ if(Lib.isArrayOrTypedArray(target) && target.length === 0) {


@etpinard Thanks for the PR.
Cool!
What about handling a case like this: [ [ ] ]?
See codepen.

Good call. Thanks for the review!

@archmoj this turned out to be a bit more of a project than anticipated.
See 3a5a4c2 - which makes things work for all trace types except scattercarpet and in some world-calendar scenario. I'll try to get those two cases fixed before 1.48.0, but in the meantime here's "a good chuck" of the solution.

which makes things work for all trace types except scattercarpet and in some world-calendar scenario.
Now fixed by b643238 and 40c8abc
Making this PR ready for a second 👁️

... after transform _module.calc loop - that way filter transforms that remove all data coordinates don't result in errors - of all the mocks listed in mock_lists.js, only 'scattercarpet' and 'world-cals' still error out (wip)

archmoj · 2019-05-08T16:27:03Z

@etpinard you mentioned a possible conflict on funnel PR here:
#3817 (comment)

etpinard · 2019-05-08T19:39:48Z

@etpinard you mentioned a possible conflict on funnel PR here:
#3817 (comment)

and the fix is in 84c3606

archmoj · 2019-05-08T19:53:14Z

Looks very good.
Should we handle [ [ ] ] case also?

etpinard · 2019-05-08T19:56:30Z

Should we handle [ [ ] ] case also?

It is handled as it results in trace._length=0

archmoj · 2019-05-08T20:03:38Z

Should we handle [ [ ] ] case also?
It is handled as it results in trace._length=0

Could you please provide a codepen example for that?
Thanks.

etpinard · 2019-05-08T20:10:51Z

Could you please provide a codepen example for that?

https://codepen.io/etpinard/pen/dEYQXW

archmoj · 2019-05-08T20:14:56Z

Could you please provide a codepen example for that?
https://codepen.io/etpinard/pen/dEYQXW

Thanks. Now could you please check this one.
Is that the expected behaviour?

etpinard · 2019-05-08T20:49:26Z

Is that the expected behaviour?

I think so. It's not great, but I think traces with _length===0 should still get a legend item as mentioned in #2908 (comment)

archmoj · 2019-05-08T20:53:02Z

Excellent.
[ [ 💃 ] ]

set enabled:false when filter target array is empty
66645ed
- see #2908 for more info, - this fixes many potential problems downstream

etpinard added bug something broken status: reviewable labels Apr 12, 2019

archmoj reviewed Apr 12, 2019
View reviewed changes

etpinard added 3 commits April 26, 2019 14:37

Merge branch 'master' into filter-enabled-false-when-empty-target-array
1e57928

treat trace w/ _length===0 as if they were visible:false
3a5a4c2
... after transform _module.calc loop - that way filter transforms that remove all data coordinates don't result in errors - of all the mocks listed in mock_lists.js, only 'scattercarpet' and 'world-cals' still error out (wip)

fixup tests post _length===0 commit
afad530

etpinard added this to the v1.48.0 milestone Apr 26, 2019

etpinard added 2 commits April 29, 2019 10:30

get filter transform to work with scattercarpet traces
b643238

don't try to cleanDate coming from autorange
40c8abc

etpinard mentioned this pull request Apr 29, 2019
Funnel traces #3817
Merged

etpinard added 2 commits May 8, 2019 14:58

Merge branch 'master' into filter-enabled-false-when-empty-target-array
a20cb01

fixup funnel test for new _length===0 logic
84c3606

etpinard merged commit d6da48e into masterMay 8, 2019

etpinard deleted the filter-enabled-false-when-empty-target-array branch May 8, 2019 20:54

Uh oh!

Uh oh!

Conversation

etpinard commented Apr 12, 2019

Uh oh!

archmojApr 12, 2019

Choose a reason for hiding this comment

Uh oh!

etpinardApr 15, 2019

Choose a reason for hiding this comment

Uh oh!

etpinardApr 26, 2019

Choose a reason for hiding this comment

Uh oh!

etpinardApr 29, 2019

Choose a reason for hiding this comment

Uh oh!

archmoj commented May 8, 2019

Uh oh!

etpinard commented May 8, 2019

Uh oh!

archmoj commented May 8, 2019

Uh oh!

etpinard commented May 8, 2019

Uh oh!

archmoj commented May 8, 2019

Uh oh!

etpinard commented May 8, 2019

Uh oh!

archmoj commented May 8, 2019

Uh oh!

etpinard commented May 8, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

archmoj commented May 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

etpinard commented May 8, 2019•
edited
Loading