evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

adds stepping prop for line & area chart. #1098

Closed commitsovercoffee closed 8 months ago

commitsovercoffee commented 9 months ago

Checklist

Description

Attempts to resolve #937

Line Chart Area Chart
line-chart area-chart

Usage

stepping defines whether to show as a step line. It can be true, false. Or 'start', 'middle', 'end'. Which will configure the turn point of step line. Reference

<AreaChart data={simpler_bar} x=year y=value series=country stepping={true}/>
<LineChart data={simpler_bar} x=year y=value series=country stepping={"start"}/>

Please let me know if any changes are required.

changeset-bot[bot] commented 9 months ago

πŸ¦‹ Changeset detected

Latest commit: 772dbf3f9c69cadc5ab187c3af3125b278e1dee9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ----------------------------- | ----- | | @evidence-dev/core-components | Patch | | @evidence-dev/components | Patch | | @evidence-dev/evidence | Patch | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
evidence-docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Aug 30, 2023 9:18pm
netlify[bot] commented 9 months ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit 798c03958e70305fe7e2df301b74ecc13e7aa01f
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/64d7b1f66f238e0008cb5cb2
Deploy Preview https://deploy-preview-1098--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 9 months ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit 772dbf3f9c69cadc5ab187c3af3125b278e1dee9
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/64efb108590b9b0008d0541d
Deploy Preview https://deploy-preview-1098--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

commitsovercoffee commented 9 months ago

While working on this, I noticed that the symbolSize or MarkerSize is very small for area chart compared to other charts. Is that an issue or is it by design ? πŸ€”

Line Chart Area Chart
line-size area-chart

As shown above, in the line chart it is clear where we are pointing at any instance as the dot is clearly visible on the line. But such is not the case for the area graph.

hughess commented 9 months ago

@commitsovercoffee this is great, thanks!

What's the default position of the step if you set it to true? (i.e. start, middle, end)

There's a couple of minor things I would change for the prop:

hughess commented 9 months ago

While working on this, I noticed that the symbolSize or MarkerSize is very small for area chart compared to other charts. Is that an issue or is it by design ? πŸ€”

As shown above, in the line chart it is clear where we are pointing at any instance as the dot is clearly visible on the line. But such is not the case for the area graph.

Good catch! I think those should be the same. Feel free to open an issue for that one, or add it in a PR if you're interested!

commitsovercoffee commented 9 months ago

To keep the syntax looking as simple as possible, make the prop accept strings rather than requiring the curly braces, so it would look like <LineChart step=true />

The markers prop in Line.svelte is a good example of that

@hughess I see that markers prop ( sets to true/false ) & another prop markerShape defines the shape. Shall I do the same ? As in ...

Is this approach good ?

hughess commented 9 months ago

@commitsovercoffee that sounds like a good idea. I'd call the second prop stepPosition.

Can we make the default "end"? I think that makes the most sense since if there's a spike, it would appear at the specific point rather than the point before

commitsovercoffee commented 9 months ago

@hughess I have incorporated the changes you asked :)

Reference Syntax :

<LineChart data={simpler_bar} x=year y=value series=country step=true />
<LineChart data={simpler_bar} x=year y=value series=country step=true stepPosition=start/>

Reference Image :

image

hughess commented 9 months ago

This looks perfect!

Last thing to get this in is just docs - can you add this to the line and area docs? I think it’ll be an extra 2 rows in the props table for each of those charts, plus a new example for each (like you’ve got in example-project).

commitsovercoffee commented 9 months ago

@hughess I see the images in examples are all in svg format. How do I save a chart in svg ?

archiewood commented 9 months ago

@hughess is your man for this. Otherwise I have no issue with png

hughess commented 9 months ago

PNG is good!

ItsMeBrianD commented 8 months ago

@archiewood anything else needed here?

archiewood commented 8 months ago

No, looks great