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

Add echarts custom options #1366

Closed hughess closed 5 months ago

hughess commented 5 months ago

Description

Adds 2 props to chart components:

Remaining Issues

Checklist

changeset-bot[bot] commented 5 months ago

⚠️ No Changeset found

Latest commit: 9da524d05b14a58afd819a8935010886dae37141

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

netlify[bot] commented 5 months ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit 9da524d05b14a58afd819a8935010886dae37141
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/65696eb6d8a8d60008dbde11
Deploy Preview https://deploy-preview-1366--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.

vercel[bot] commented 5 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 Dec 1, 2023 5:28am
archiewood commented 5 months ago

Had a quick play with this.

Overall

Super powerful. Tried something @jdimmerman was asked about recently, and worked pretty nicely.

<AreaChart
    data={orders_by_category}
    x=month
    chartAreaHeight=200
    echartsOptions={{ xAxis: { axisLabel: { rotate: 90, interval: 0 } } }}
/>

CleanShot 2023-11-30 at 13 37 33@2x

Thoughts

mcrascal commented 5 months ago

This is really cool

hughess commented 5 months ago

@archiewood I adjusted the indent to 3 spaces - initially I threw in 10 for some reason.

I agree on the code block length, but thinking because it's just a dev mode helper leave it for now. Do you think we should fix it now?

Agreed on the concerns - I was thinking this should be a separate page in the docs, which we can link to from each chart type. That way we can include a bit more of a warning/explanation

archiewood commented 5 months ago

Do you think we should fix it now?

no need if not easy

Agreed on the concerns - I was thinking this should be a separate page in the docs, which we can link to from each chart type. That way we can include a bit more of a warning/explanation

sounds very sensible

hughess commented 5 months ago

Couple other tricky things about this atm:

 echartsOptions={{
        series: [
            {},
            {},
            {symbol:'triangle'}
        ]
    }}
hughess commented 5 months ago

One other gotcha with this - if you're using a horizontal chart swapXY=true, the axis you need to make changes on will be flipped in the echarts options. The y-axis in Evidence would be referenced with xAxis in ECharts and vice versa

hughess commented 5 months ago

Last thing I noticed - it seems to not be possible to change the series type using this approach. I've created an issue with echarts for this: https://github.com/apache/echarts/issues/19349

archiewood commented 5 months ago

should / are we able to error handle the eCharts options if you pass an invalid config?

archiewood commented 5 months ago

I am writing some rudimentary docs for this