Skip to content

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

Merged
kurkle merged 1 commit into
chartjs:masterfrom
charlesmass2:test_above_below_nulls
Jan 1, 2022
Merged

kurkle merged 1 commit into
chartjs:masterfrom
charlesmass2:test_above_below_nulls

Conversation

@charlesmass2

@charlesmass2 charlesmass2 commented Dec 26, 2021

Copy link
Copy Markdown
Contributor

Fixes #10023

Added a test in the boundary folder

@charlesmass2
charlesmass2 force-pushed the test_above_below_nulls branch from 2e65fc9 to 7eb41f6 Compare December 26, 2021 18:01
@LeeLenaleee LeeLenaleee added this to the Version 3.7.1 milestone Dec 26, 2021
@kurkle

kurkle commented Dec 26, 2021

Copy link
Copy Markdown
Member

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

@charlesmass2

charlesmass2 commented Dec 26, 2021

Copy link
Copy Markdown
Contributor Author

@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)

@kurkle kurkle left a comment

Copy link
Copy Markdown
Member

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

kurkle commented Jan 1, 2022

Copy link
Copy Markdown
Member

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