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.

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

Add value labels to charts #1255

Merged
merged 14 commits into from Oct 14, 2023
Merged

Add value labels to charts #1255

merged 14 commits into from Oct 14, 2023

Conversation

hughess
Copy link
Member

@hughess hughess commented Oct 12, 2023

Chart Value Labels

This PR adds value labels to bar, line, and area charts.

CleanShot 2023-10-12 at 09 44 36@2x

Labels are turned off by default, but can be added by setting labels=true

This also includes a variety of props to format labels:

  • labelFmt: works in the same way as our other fmt props and will override any other formats set
  • labelColor: can pass a colour string in RGB, hexadecimal, CSS colour name, or "inherit" to take the colour of the series
  • labelPosition:
    • Bar
      • outside
      • inside
    • Line / Area
      • above
      • below
      • middle
  • labelSize: font size for labels

By default, label overlap is avoided, but there is an option to show everything:

  • showAllLabels: boolean

In stacked bar charts, we’ve added a stack total label which appears above the bars. This can be turned off with:

  • stackTotalLabel: boolean

Defaults

  • labelColor: according to the colour of the series (ECharts manages colour contrast)
  • labelFmt: same format as used for y column
  • labelSize: 11pt, 1 smaller than axis labels
  • labelPosition:
    • Single series bar: outside
    • Stacked bar: inside, with stack total label outside
    • Grouped bar: outside
    • Line: above
    • Area: above
  • showAllLabels: false
  • stackTotalLabel: true

Limitations

On stacked bar charts, the total label does not recalculate when hiding series by clicking on the legend. For now, I have turned off that type of interaction when a stacked total is present - otherwise, you could deselect a series and have the wrong total sitting on top of the bars.

This PR doesn’t include value labels for scatter plots, bubble charts, or histograms.

Scatter/bubble labels are up next, but involve some challenges because the approach will include the ability to pass a specific column to the label.

One issue that is unsolved here is the legend hover state. When you hover over a series in a legend, all labels will appear on the hovered series, including any overlap or conflicts. I've opened an issue with ECharts about this: apache/echarts#19192

Questions

  • Is “label” the right term here, or should these be called “valueLabel”?
    • Value labels is clearer, but makes all the props longer

Notes on changes to charting library

  • This PR upgrades ECharts to 5.4.3 - this was needed to get a bugfix for line chart labels
  • The legend has been changed to accept columns specified in each child chart component (e.g., Line.svelte) - this is because the hidden stack total series could not be removed from the legend otherwise
  • Fixed MarkerSize is very small for area chart compared to other charts. #1108

Checklist

  • For UI or styling changes, I have added a screenshot or gif showing before & after
  • I have added a changeset

@changeset-bot
Copy link

changeset-bot bot commented Oct 12, 2023

🦋 Changeset detected

Latest commit: a87a9cb

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

This PR includes changesets to release 5 packages
Name Type
@evidence-dev/component-utilities Minor
@evidence-dev/core-components 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

@netlify
Copy link

netlify bot commented Oct 12, 2023

Deploy Preview for evidence-development-workspace ready!

Name Link
🔨 Latest commit a87a9cb
🔍 Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/6529d7b1e77acd0008a2de91
😎 Deploy Preview https://deploy-preview-1255--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
Copy link

vercel bot commented Oct 12, 2023

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 Oct 13, 2023 11:50pm

@hughess
Copy link
Member Author

hughess commented Oct 12, 2023

Text Flickering Issue

Noticed this on Safari and checked out all the browsers I have. This reminds me of a previous issue I had with outlined text, where having a thin outline around characters caused some browsers to not render it correctly:

CleanShot 2023-10-12 at 10 12 42

Mac Desktop

  • Chrome: Works
  • Firefox: Works
  • Safari: Issue

Windows Desktop

  • Chrome: Works
  • Firefox: Works
  • Edge: Works

Mobile

  • Chrome: Issue
  • Firefox: Issue
  • Safari: Issue

Printing/PDF seems okay on all browsers as far as I can tell, it's just the hover interaction that seems to cause this

apache/echarts#19199

@hughess
Copy link
Member Author

hughess commented Oct 12, 2023

Update on flickering issue

This is caused by the textBorder option in ECharts. We could turn it off, but it's very helpful in managing the contrast in labels compared to their backgrounds.

This issue only happens with the SVG renderer in ECharts. Switching to Canvas renderer solves the problem on desktop browsers. Still need to confirm for mobile browsers.

Should we change to the canvas renderer?

@ItsMeBrianD @csjh @mcrascal @archiewood

Initially I picked the SVG renderer because it maintains 100% crisp visuals when you zoom in vs. blurry with Canvas.

Here's ECharts' description of choosing a renderer:

Generally, Canvas is more suitable for charts with a large number of elements (heat map, large-scale line or scatter plot in geo or parallel coordinates, etc.), and with visual effect. However, SVG has an important advantage: It has less memory usage (which is important for mobile devices) and won't be blurry when zooming in.

I'm not sure how to check the performance/memory impacts of making the change.

Does anyone have concerns with changing to the canvas renderer?

@csjh
Copy link
Member

csjh commented Oct 12, 2023

I think some features are locked to certain renderers, though I'm looking through the docs now and the only one they mention is renderToSVGString (which we don't use)

@hughess
Copy link
Member Author

hughess commented Oct 12, 2023

UI test fails after changing to canvas because the test looks for a <g> element. Have tested the functionality and it's working

@hughess
Copy link
Member Author

hughess commented Oct 13, 2023

Realized this is going to need to work with the secondary y-axis as well.

That may mean changing the prop names to align to an axis. i.e.:

  • yLabels
  • yLabelFmt
  • yLabelPosition
  • yLabelColor
  • yLabelSize
  • yShowAllLabels
  • yStackTotalLabel
  • y2Labels
  • y2LabelFmt
  • y2LabelPosition
  • y2LabelColor
  • y2LabelSize
  • y2ShowAllLabels
  • y2StackTotalLabel

This may be confusing when compared to yAxisLabels and y2AxisLabels, so maybe adding "value" to these props is a good idea despite the longer names:

  • yValueLabels
  • yValueLabelFmt
  • yValueLabelPosition
  • yValueLabelColor
  • yValueLabelSize
  • yShowAllValueLabels
  • yStackTotalLabel
  • y2ValueLabels
  • y2ValueLabelFmt
  • y2ValueLabelPosition
  • y2ValueLabelColor
  • y2ValueLabelSize
  • y2ShowAllValueLabels
  • y2StackTotalLabel

@archiewood
Copy link
Member

archiewood commented Oct 13, 2023

do you need to be able to control the two axes labels independently?

If you had two series on the same axis, you wouldn't be able to to that

Or could they both be controlled by one set of props?

labels
labelFmt
labelPosition
labelColor
labelSize
showAllLabels
stackTotalLabel

The only one I can see a reason to have independent control over is the fmt

@hughess
Copy link
Member Author

hughess commented Oct 13, 2023

I think fmt would be the most common, but I could also see position - e.g., you have a bar and want the labels to be positioned "inside" vs. "above" for the line on the other axis

@archiewood
Copy link
Member

archiewood commented Oct 13, 2023

postion

you can't control position in a multi series line chart independently, so I would be happy to not support that.

fmt

with two axes there is a decent chance the formats would be different.

I'd probably have:

labelFmt - adjusts both
yLabelFmt - adjusts first axis, overrideing labelFmt if specified
y2LabelFmt - adjusts second axis, overrideing labelFmt if specified

@hughess
Copy link
Member Author

hughess commented Oct 13, 2023

good point - i had already forgotten that y2 only works with lines now. Agree with those

@hughess hughess merged commit 2372a36 into main Oct 14, 2023
17 of 18 checks passed
@hughess hughess deleted the value-labels branch October 14, 2023 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MarkerSize is very small for area chart compared to other charts.
3 participants