-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[charts] Allows to connect nulls #10803
Conversation
Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Co-authored-by: Flavien DELANGLE <flaviendelangle@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement! 💙 💯
Some other thoughts:
- I'm not sure if this is the best prop name, but maybe you have already done competitor analysis and this is also used by some other libraries
- Wouldn't having
connectNulls
opt-out instead of opt-in make more sense? Or are others also approaching this behavior the same? 🤔
No idea why argos fails
Just ignore those silly pixel diffs. IMHO, it's mainly a problem, because we are making quite low-quality screenshots and in this case, it's SVG, rather than regular HTML elements, maybe that has extra rendering inaccuracy issues. 🤷
I picked Seems echarts has the same naming: https://echarts.apache.org/en/option.html#series-line.connectNulls
From what I see others are doing the same. The reason is that if you provide |
Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Thanks for the reply. |
Co-authored-by: Lukas <llukas.tyla@gmail.com> Co-authored-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Follow up of #10801
Most of the files modified are demos added to reproduce the recharts examples and be sure it works as expected.
Idea
The overall idea is that if
connectNulls
is used by the user, instead of using the d3 functiondefined
, we filter the data to remove all thenull
elements. Such that d3 sees only the valid one and then does the interpolation between them.Test
No idea why argos fails. I copy/paste the HTML of the legend from the preview and from the master deployment. It's exactly the same 🤷♂️