Skip to content

Handle NaN positions in funnel & waterfall and skip connectors over rangebreaks - #4663

Merged
archmoj merged 4 commits into
masterfrom
rangebreaks-waterfall-connectors
Mar 19, 2020
Merged

Handle NaN positions in funnel & waterfall and skip connectors over rangebreaks #4663
archmoj merged 4 commits into
masterfrom
rangebreaks-waterfall-connectors

Conversation

@archmoj

@archmojarchmoj commented Mar 18, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Follow up of #4614,
this PR fixes#4631 for funnel & waterfall.
As well it fixes error in drawing a connector over a rangebreak.

Before | After.

TODO:

  • fix waterfall and add test
  • fix funnel and add test

@plotly/plotly_js

@archmojarchmoj added this to the v1.53.0 milestone Mar 18, 2020
@alexcjohnson

alexcjohnson commented Mar 19, 2020

Copy link
Copy Markdown
Collaborator

Looks good, nice find! This is for when the connector enters or exits the break, what if the connector completely crosses the break? Does that already work?

@archmoj

archmoj commented Mar 19, 2020

Copy link
Copy Markdown
ContributorAuthor

Looks good, nice find! This is for when the connector enters or exits the break, what if the connector completely crosses the break? Does that already work?

Already works: before | after

@archmojarchmoj mentioned this pull request Mar 19, 2020
Closed
@archmojarchmoj changed the title Handle waterfall connector lines with rangebreaksSkip waterfall connector lines over rangebreaks and handle NaN positions in funnel & waterfall Mar 19, 2020
@archmojarchmoj changed the title Skip waterfall connector lines over rangebreaks and handle NaN positions in funnel & waterfall Handle NaN positions in funnel & waterfall and skip connectors over rangebreaks Mar 19, 2020
@archmoj

archmoj commented Mar 19, 2020

Copy link
Copy Markdown
ContributorAuthor

Please wait.
Still there is an error in this demo.

@archmoj

archmoj commented Mar 19, 2020

Copy link
Copy Markdown
ContributorAuthor

Please wait.
Still there is an error in this demo.

Resolved. I was using the old bundle!

@archmoj

archmoj commented Mar 19, 2020

Copy link
Copy Markdown
ContributorAuthor

@alexcjohnson this is PR is ready for second review.

alexcjohnson
alexcjohnson approved these changes Mar 19, 2020
View reviewed changes

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! 💃

@archmoj
archmoj merged commit 0ad049c into masterMar 19, 2020
@archmoj
archmoj deleted the rangebreaks-waterfall-connectors branch March 19, 2020 18:21
@snyk-botsnyk-bot mentioned this pull request Apr 23, 2020
Open
@snyk-botsnyk-bot mentioned this pull request May 1, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexcjohnsonalexcjohnsonalexcjohnson approved these changes

Assignees

No one assigned

Labels

bugsomething broken

Projects

None yet

Milestone

v1.53.0

Development

Successfully merging this pull request may close these issues.

Bars with NaNs as positions lead to console errors

2 participants

@archmoj@alexcjohnson