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

Reference lines and reference areas #858

Closed hughess closed 1 year ago

hughess commented 1 year ago

Description

This PR adds reference lines and areas to charts.

For now, these are separate components: ReferenceLine and ReferenceArea, which are used by inserting them into a slot within a chart. For example:

<LineChart
   data={sales_data}
   x=date
   y=sales
>
   <ReferenceLine yVal=120 label="Target"/>
</LineChart>

These can be applied by hardcoding values (e.g., if you want a horizontal line at y=50) or with a dataset (e.g., you have a table of events or time ranges you want to include in a chart to add context to the viz).

Reference Line

Props

Required:

Optional:

CleanShot 2023-05-07 at 12 26 31@2x
<LineChart 
    data={daily_complaints} 
    x=date 
    y=number_of_complaints 
    title="Complaint Calls to Austin 311"
>
    <ReferenceLine yVal=2500 label=Threshold/>
</LineChart>

Reference Area

Props

Required:

Optional:

CleanShot 2023-05-06 at 15 23 05@2x
<LineChart 
    data={daily_complaints} 
    x=date 
    y=number_of_complaints 
    title="Complaint Calls to Austin 311"
>
    <ReferenceArea data={annotate} xMin=start_date xMax=end_date label=label/>
</LineChart>

Use Cases

This opens up some pretty exciting use cases for adding relevant context to data viz.

CleanShot 2023-05-07 at 12 36 50@2x

To Do

Deferred for future upgrades

Issues to be solved

Error Handling

Implemented using the ErrorChart component.

Covers these scenarios:

CleanShot 2023-05-09 at 15 56 10@2x

Checklist

changeset-bot[bot] commented 1 year ago

πŸ¦‹ Changeset detected

Latest commit: d6a8405f64d6f8a786218df4d0f1cdf51f0f596c

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/preprocess | Minor | | @evidence-dev/components | Minor | | @evidence-dev/evidence | Major | | 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 1 year 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 May 18, 2023 5:17pm
netlify[bot] commented 1 year ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit d6a8405f64d6f8a786218df4d0f1cdf51f0f596c
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/64665d88f17eef000800c1b8
Deploy Preview https://deploy-preview-858--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 settings.

hughess commented 1 year ago

The last outstanding issue that may need to be tackled before merging is the default styling of labels. The basic labels look fine, but when a label is overtop of lines/bars/etc it doesn't look right.

Options are adding a white outline on the text or adding a background colour to the label (either full opacity or with some transparency).

Given the number of permutations here, it might be worth merging with a small white text outline as a starting point, and spending time to solve this in a more general sense once we have a bit of user feedback.

hughess commented 1 year ago

Update: I've added a white text outline by default on reference line labels, but have also included a prop for labelBackground which if set to true, will include a white semi-transparent background behind the label

hughess commented 1 year ago

Thanks @archiewood.

I think I want more precisions on target line label than the axis. Should I be able to configure this?

What do you mean by precisions?

archiewood commented 1 year ago

I was trying to achieve two lines where the labels didn't overlap

By more precision (typo), here I meant I wanted the format to show $3,800 not $4k.

hughess commented 1 year ago

Ahh that's a great point. Do you think a good solution to that might be a format override prop like valueFmt or similar?

archiewood commented 1 year ago

Yes I think so. Or accept the format function as I tried and failed to do

hughess commented 1 year ago

I've added value format control to the list of future improvements.

Just pushed some simplification to the props and label positions

hughess commented 1 year ago

Ran into 2 last issues on this:

Text Borders

Using a text border on the labels causes an issue with rendering in Safari. This is an echarts bug - I've opened an issue here: https://github.com/apache/echarts/issues/18609

Bar Charts with Numeric or Date x-axis

Using a reference area on a bar chart where the x-axis is numeric or date, the are will only go to the specific point on the x-axis and will not cover the entire bar:

CleanShot 2023-05-12 at 08 41 00@2x

This is because the area is lined up to the exact point on the axis.

There are 2 possible workarounds:

1. Add extra padding to the dates to be used to create the area.

For example, this chart is set with xMin='2020-12-15' and xMax='2021-07-15'

CleanShot 2023-05-12 at 08 42 36@2x

2. Change x-axis type to category

This is not ideal, as it then removes automatic formatting of the x-axis labels. I would say this is not a viable workaround

Other Alternatives

There are a couple of ways to work around this on our side by extracting and adjusting the coordinates from echarts. These may be worth doing if the above workaround isn't intuitive.

archiewood commented 1 year ago

@hughess as discussed I think we should probably leave the behaviour as is for numeric axes highlighting.

It's kind of a difficult problem as barcharts are sort of a discrete chart type, but are often used in a continuous setting (eg dates, numeric ranges).

Given the workarounds exist (workaround 1 in particular), I think until we are clear on what users need here, leaving as is makes sense

hughess commented 1 year ago

There are only 2 things blocking this PR from merging:

  1. Format test failing, but this is only for markdown pages - @ItsMeBrianD wanted to double check that this is okay
  2. Docs - since this includes docs changes, assume we should wait until we're close to release?
archiewood commented 1 year ago

Removed formatting checks for markdown files in the example project +page.md files.