Skip to content
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

Fix using above/below filling option with discontinuous lines #10024

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

charlesmass2
Copy link
Contributor

@charlesmass2 charlesmass2 commented Dec 26, 2021

Fixes #10023

Added a test in the boundary folder

@kurkle
Copy link
Member

kurkle commented Dec 26, 2021

What does spanGaps: true do in this case? And is there a reason it's not used?

@charlesmass2
Copy link
Contributor Author

charlesmass2 commented Dec 26, 2021

@kurkle spanGaps: true would draw a line from the last point of a segment, to the first point of the next segment.
So if one wants to have proper discontinuity in the filling of a line chart, it's not possible with above/below.
Basically, this PR reproduces the behavior of the original commit 18e3bc0 (which was before the 3.0.0 update)

EDIT : Actually, if you'd like to see the difference, you can set spanGaps: true in the two fixtures I added. It might add some clarity 😄

EDIT 2: On our end, we use this to display increasing sets of values over a time period. For instance, increasing values over 1 week with a chart range of 3 weeks. Which would give something like that (once the bug is fixed):
download (1)

Same config before this bugfix (on master)

download (3)

Same config with `spanGaps: true`

download (2)

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Sorry for taking a while before finding the time to properly review.
I had some doubts about having the discontinuity at different spots for dataset and the target, but that seems to work as expected too. Clip is not "grounded" on those spots, but its handled by _fill, so there is no problem.

Good job! 👍

@kurkle
Copy link
Member

kurkle commented Jan 1, 2022

To visualize this change, clip areas colored for above-below-line-null.json

before
image

after
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filler plugin can have the wrong behavior when using null values
3 participants