Skip to content

fix(bars): add xAxis tickFormatter check to value labels#1390

Closed
rshen91 wants to merge 9 commits intoelastic:masterfrom
rshen91:tickFormat
Closed

fix(bars): add xAxis tickFormatter check to value labels#1390
rshen91 wants to merge 9 commits intoelastic:masterfrom
rshen91:tickFormat

Conversation

@rshen91
Copy link
Copy Markdown
Contributor

@rshen91 rshen91 commented Sep 20, 2021

Summary

The tickFormat on the data values over bars now will correctly display when charts are rotated.

Rotations are accounted for in:

  • tooltip headers
  • tooltip bodies
  • data values over bar geometries

Details

Issues

Fixes #1387

Checklist

  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated

@rshen91
Copy link
Copy Markdown
Contributor Author

rshen91 commented Sep 21, 2021

@markov00 I could use some clarification on expected behavior of rotations. In https://elastic.github.io/elastic-charts/?path=/story/bar-chart--with-value-label-advanced&globals=theme:light it seems like the value labels over the bars do respect the tickFormat with rotations. I've added the Data Value Test story to try and gain some insight in another example where I was seeing the issue. Let me know if I'm misunderstanding the problem in the With Label Value Advanced story.

I have the story working in the latest commit c46db3f where the chart can be rotated 90 degrees and by toggling the 'turn off special tickFormat' knob the data values on the bars are changing correctly. However, there's the issue of the ordinal axis and the tickFormat being applied to it leading to the NanK. I would love to make sure I'm implementing the fix correctly and not get into the axes in axis_utils.ts unless that's part of the fix that's needed. Is that a separate issue? Thank you!

@rshen91
Copy link
Copy Markdown
Contributor Author

rshen91 commented Sep 21, 2021

Adding a video to show more context:

Sep-21-2021.13-30-03.mp4

@markov00
Copy link
Copy Markdown
Collaborator

markov00 commented Sep 22, 2021

Sure, so in the Bar Chart With Value Label Advanced story the code looks like this:

<Axis id="bottom" position={Position.Bottom} title="Bottom axis" showOverlappingTicks />
<Axis id="left2" title="Left axis" position={Position.Left} tickFormat={(d: any) => Number(d).toFixed(2)} />

We are applying a toFixed(2) number formatting to the Left (aka Y) axis and with rotation=0 you can notice the formatting applied both on the axis tick labels and on the bar values.
Screenshot 2021-09-22 at 15 15 31

Now, let's see the 90 degree rotated version: the Left axis, in this case, is our cartesian X-axis (is still on the left of the screen but due to the rotation it semantically changed its role and become an x-axis). So now, bar values should follow the Bottom axis (Y-axis) formatting convention, which is defined in the code as no formatting at all. So I'm expecting to see whole numbers, with and without decimal places, the same as I'm seeing in the tooltip.

From what I can see, the getAxesSpecForSpecId doesn't consider the rotation when it defines an axis to be an X or Y cartesian axis.
Screenshot 2021-09-22 at 15 21 54

The issue is way more visible if you consider a common use case, were the user invert the axis formatter depending on the rotation.

const yFormatter = (d: any) => Number(d).toFixed(2);

<Axis id="bottom" position={Position.Bottom} tickFormat={rotation=== 90 ? yFormatter : undefined} />
<Axis id="left" position={Position.Left} tickFormat={rotation=== 0 ? yFormatter : undefined} />

@rshen91
Copy link
Copy Markdown
Contributor Author

rshen91 commented Oct 4, 2021

Closing and opening in a new PR

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.

Wrong tickFormat applied to the value labels

2 participants