Rounded Tick Values in Declarative charts #33619
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
change/@fluentui-react-charting-ea816697-9232-4117-8d82-fd6fbacb2acb.json
Show resolved
Hide resolved
Anush2303
changed the base branch from
master
to
user/atisjai/stronglyTypePlotly
…ative/roundedTicks
packages/charts/react-charting/src/components/DeclarativeChart/PlotlySchemaAdapter.ts
Outdated
Show resolved
Hide resolved
|
@Anush2303 you can add few tests for the changes |
…ative/roundedTicks
Okay sure. |
|
Lets get the playwright screenshot diffs for this change |
| @@ -422,6 +422,7 @@ export class CartesianChartBase | |||
| this.isIntegralDataset, | |||
| false, | |||
| this.props.supportNegativeData!, | |||
| this.props.roundedTicks!, | |||
| const finalYmin = supportNegativeData | ||
| const finalYmax = roundedTicks ? yMaxValue : tempVal > yMaxValue ? tempVal : yMaxValue!; | ||
| const finalYmin = roundedTicks | ||
| ? yMinValue |
There was a problem hiding this comment.
I think roundedTicks prop is capable of handling the negative y values also. It is taking into account the minimum and maximum y values and passing that from the adapter itself.
| ): number[] { | ||
| if (roundedTicks) { | ||
| const ticks = d3Ticks(minVal, maxVal, splitInto); |
There was a problem hiding this comment.
In the documentation, they mention that approximately count+1 ticks will be produced. In our case, it is working well with 'splitInto'.
For eg. d3.ticks(1, 9, 5) // [2, 4, 6, 8] . Count is 5, but in result it gives only 4.
| ): number[] { | ||
| if (roundedTicks) { | ||
| const ticks = d3Ticks(minVal, maxVal, splitInto); | ||
| const gap = ticks.length > 1 ? ticks[1] - ticks[0] : 1; |
There was a problem hiding this comment.
It is 1 for the case, ticks.length = 1. And we will be having ticks.length=1 when there is only 1 y value. So in that case actually, the default value doesn't matter. I have set it to 1 just to handle the edge case.
| ticks.push(ticks[ticks.length - 1] + gap); | ||
| } | ||
| return ticks; | ||
| } |
There was a problem hiding this comment.
Could you add UTs for this change.
Move this logic in a separate function
| const ticks = d3Ticks(minVal, maxVal, splitInto); | ||
| const gap = ticks.length > 1 ? ticks[1] - ticks[0] : 1; | ||
| if (ticks[0] !== minVal) { | ||
| ticks.unshift(ticks[0] - gap); |
There was a problem hiding this comment.
Does nice ticks solve your purpose.
https://d3js.org/d3-array/ticks#nice
| ): number[] { | ||
| if (roundedTicks) { |
|
The target branch is already merged. I will be creating a new PR from master with all the comments taken into account. |
New Behavior
Related Issue(s)