Added continuousLine prop to line chart #475
Conversation
There was a problem hiding this comment.
Thank you for your contribution! In this library I try to introduce as few "custom" props as possible, meaning that I instead try to expose the underlying building blocks, e.g defined so that the user can do more powerful things and the implementation stays clean. Creating a prop for each specific use case would very quickly bloat this library and make the implementation very hard to follow.
New properties also need to thoroughly be documented in the README.
| const { curve, continuousLine } = this.props | ||
| let line | ||
| if (continuousLine) { | ||
| line = shape | ||
| .line() | ||
| .x((d) => x(d.x)) | ||
| .y((d) => y(d.y)) | ||
| .curve(curve)(data.filter((item) => item.y)) | ||
| } else { | ||
| line = shape | ||
| .line() | ||
| .x((d) => x(d.x)) | ||
| .y((d) => y(d.y)) | ||
| .defined((item) => typeof item.y === 'number') | ||
| .curve(curve)(data) | ||
| } |
There was a problem hiding this comment.
Wouldn't this work?
Basically it exposes the defined function to user land, allowing them to implement it anyway they see fit
| const { curve, continuousLine } = this.props | |
| let line | |
| if (continuousLine) { | |
| line = shape | |
| .line() | |
| .x((d) => x(d.x)) | |
| .y((d) => y(d.y)) | |
| .curve(curve)(data.filter((item) => item.y)) | |
| } else { | |
| line = shape | |
| .line() | |
| .x((d) => x(d.x)) | |
| .y((d) => y(d.y)) | |
| .defined((item) => typeof item.y === 'number') | |
| .curve(curve)(data) | |
| } | |
| const { | |
| defined = (item) => typeof item.y === 'number' , | |
| continuousLine | |
| } = this.props | |
| const shape = | |
| .line() | |
| .x((d) => x(d.x)) | |
| .y((d) => y(d.y)) | |
| .defined(defined) | |
| .curve(curve)(data) |
There was a problem hiding this comment.
Hi, can you explain how can I use defined function for this situation?
There was a problem hiding this comment.
Hi, my suggestion above shows that defined is exposed as a prop. Meaning that we can have a default value (the one it is currently) and that the user can override it how they see fit
There was a problem hiding this comment.
I tried to implement the defined function for this specific use case but I failed. Do you have any idea?
That would help me to solve the main problem.
There was a problem hiding this comment.
If you could provide a PR with your attempt and clearly stating what isn't working I might be able to help you 👍
There was a problem hiding this comment.
Can you help me with this?
Actually I think it's impossible to handle this use case without filtering data.
Checkout this example from d3.
There was a problem hiding this comment.
I unfortunately don't have any time to assist outside of reviewing the PR. The user controls the data so I don't see why the user won't be able to filter the data? What exactly are you referring to in your example?
There was a problem hiding this comment.
@JesperLekland For example, we want to show the week, and the data is only available for Monday and Thursday. Line Chart will not be able to connect Monday and Thursday.
There was a problem hiding this comment.
it's not only .defined(defined) but also the data being fed to the chain is different.
|
Hi, I guess I'm late on the discussion but I also need this PR so want to add some comments. Difference between |
| line = shape | ||
| .line() | ||
| .x((d) => x(d.x)) | ||
| .y((d) => y(d.y)) | ||
| .curve(curve)(data.filter((item) => item.y)) |
There was a problem hiding this comment.
I think this should look like below, since data is array of line.
| line = shape | |
| .line() | |
| .x((d) => x(d.x)) | |
| .y((d) => y(d.y)) | |
| .curve(curve)(data.filter((item) => item.y)) | |
| lines = data.map((line) => | |
| shape | |
| .line() | |
| .x((d) => x(d.x)) | |
| .y((d) => y(d.y)) | |
| .curve(curve)(line.filter((item) => item.y)) |
|
@rferdosi and @JesperLekland Is this solution available for general usage, I just installed the package and still can't have a continuesLine |
Added continuousLine props to line chart to skip null items in data.
fixes: #468,