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.

Sign up for GitHub

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

Added continuousLine prop to line chart #475

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Added continuousLine prop to line chart #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions src/line-chart/line-chart-grouped.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,27 @@ import ChartGrouped from '../chart/chart-grouped'

class LineChartGrouped extends ChartGrouped {
createPaths({ data, x, y }) {
const { curve } = this.props

const lines = data.map((line) =>
shape
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))
Comment on lines +9 to +13
Copy link

Choose a reason for hiding this comment

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

I think this should look like below, since data is array of line.

Suggested change
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))

} else {
line = shape
.line()
.x((d) => x(d.x))
.y((d) => y(d.y))
.defined((item) => typeof item.y === 'number')
.curve(curve)(line)
)
.curve(curve)(data)
}

return {
path: line,
line,
}
return {
path: lines,
lines,
Expand Down
24 changes: 16 additions & 8 deletions src/line-chart/line-chart.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@ import Chart from '../chart/chart'

class LineChart extends Chart {
createPaths({ data, x, y }) {
const { curve } = this.props

const line = shape
.line()
.x((d) => x(d.x))
.y((d) => y(d.y))
.defined((item) => typeof item.y === 'number')
.curve(curve)(data)
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)
}
Comment on lines +6 to +21
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't this work?
Basically it exposes the defined function to user land, allowing them to implement it anyway they see fit

Suggested change
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)

Copy link
Author

Choose a reason for hiding this comment

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

Hi, can you explain how can I use defined function for this situation?

Copy link
Owner

@JesperLekland JesperLekland May 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

If you could provide a PR with your attempt and clearly stating what isn't working I might be able to help you 👍

Copy link
Author

@rferdosi rferdosi May 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Owner

Choose a reason for hiding this comment

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

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?

Copy link

@ALPNP ALPNP Jan 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link

@zirho zirho Dec 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

it's not only .defined(defined) but also the data being fed to the chain is different.


return {
path: line,
Expand Down
22 changes: 22 additions & 0 deletions storybook/stories/line-chart/continuous-line.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react'
import { LineChart, Grid } from 'react-native-svg-charts'

class ContinuousLineLineChartExample extends React.PureComponent {
render() {
const data = [50, 10, 40, 95, null, -24, 85, 91, 35, 53, -53, null, 50, -20, -80]

return (
<LineChart
style={{ height: 200 }}
data={data}
svg={{ stroke: 'rgb(134, 65, 244)' }}
contentInset={{ top: 20, bottom: 20 }}
continuousLine
>
<Grid />
</LineChart>
)
}
}

export default ContinuousLineLineChartExample
2 changes: 2 additions & 0 deletions storybook/stories/line-chart/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Standard from './standard'
import Partial from './partial'
import WithGradient from './with-gradient'
import Grouped from './grouped'
import ContinuousLine from './continuous-line'
import ShowcaseCard from '../showcase-card'

storiesOf('LineChart')
Expand All @@ -13,3 +14,4 @@ storiesOf('LineChart')
.add('Partial', () => <Partial />)
.add('With gradient', () => <WithGradient />)
.add('Grouped', () => <Grouped />)
.add('Continuous line', () => <ContinuousLine />)