fix(plugin-chart-echarts): invalid total label location for negative values in stacked bar chart

[justinpark]

SUMMARY

A stacked chart sets the total value only, the total label displays on top of the most latest non-null data index item.
When the data contains negative values, the stack chart places the item underneath the positive item.
Therefore the total value can be located within the stack bar when a negative value in the most latest index.
This commit fixes the extractShowValueIndexes logic by ignoring the negative values for onlyTotal option.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

Storybook

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[ktmud]

How does stacked bar chart work when a series group has a mix of negative and positive values? Is the total above the bars only the positive part or a sum of both positives and negatives?

Maybe we need to display both negative totals and positive totals separately, with positive totals on top of positive bars (if there are positive sums in a series) and negative totals below negative bars (if there are negatives).

[justinpark]

How does stacked bar chart work when a series group has a mix of negative and positive values?

As shown, the total value will be shown on top of the positive stack item for onlyTotal

[justinpark]

Maybe we need to display both negative totals and positive totals separately, with positive totals on top of positive bars (if there are positive sums in a series) and negative totals below negative bars (if there are negatives).

Displaying two total numbers can be confused. (because user expects to have one total value which means sum of positives and negatives)

++ user can highlight the each negative and positive value by the tooltip

[codecov]

Codecov Report

Merging #21032 (60a2e8e) into master (e13c5b2) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master   #21032      +/-   ##
==========================================
- Coverage   66.28%   66.28%   -0.01%     
==========================================
  Files        1770     1770              
  Lines       67497    67505       +8     
  Branches     7171     7177       +6     
==========================================
+ Hits        44738    44743       +5     
  Misses      20928    20928              
- Partials     1831     1834       +3     

Flag	Coverage Δ
javascript	52.01% <60.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...gin-chart-echarts/src/Timeseries/transformProps.ts	54.65% <ø> (ø)
...lugin-chart-echarts/src/Timeseries/transformers.ts	50.00% <50.00%> (-0.40%)	⬇️
...d/plugins/plugin-chart-echarts/src/utils/series.ts	85.08% <62.50%> (-0.90%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ktmud]

Displaying two total numbers can be confused

You can argue both ways, visually it's not very intuitive that a taller bar may have smaller total number on top of it only because it also has a negative part. But I agree the actual totals may indeed be more useful in most cases.

[kamalkeshavani-aiinside]

Here is a related issue I had reported(not for stack bars):
#20517

I think that in case of negative value, the label should be at the bottom of the bar not at the top of the bar. User would expect the label to be at the end of the bar, which would be the TOP for positive and BOTTOM for negative values. Please consider.

[justinpark]

I think that in case of negative value, the label should be at the bottom of the bar not at the top of the bar. User would expect the label to be at the end of the bar, which would be the TOP for positive and BOTTOM for negative values. Please consider

@kamalkeshavani-aiinside I understand the position for the negative values but the problem is when values are combined with positives and negatives. In this case we should choose either top or bottom because echart only accepts one position for one series.

We can design the position to BOTTOM when most of combined values are negatives.
Here is the example of this case.
https://codepen.io/airbnb/pen/zYWLbJd

Other solution is using a default position(which is inside) for positive-negative combined values for bar chart only.

Use TOP for most positives series, BOTTOM for most negative series	Use inside position for the combined case

Which option seems better?

[kamalkeshavani-aiinside]

@justinpark Thanks for the clarification. Personally, I prefer 1st option (TOP or BOTTOM based on most values), but 2nd option can be useful for others.
Can we add a checkbox to allow the user to select label position (outside/inside the bar)? This checkbox can be seen only if the users selects 'Show Values' option. Default can be outside, since it is the current behavior.

[villebro]

How does stacked bar chart work when a series group has a mix of negative and positive values? Is the total above the bars only the positive part or a sum of both positives and negatives?

Maybe we need to display both negative totals and positive totals separately, with positive totals on top of positive bars (if there are positive sums in a series) and negative totals below negative bars (if there are negatives).

@ktmud I agree that the best way to stack when positive and negative values are present is to have the negative values stacked below the line and respectively the positive ones above the line. I have a slightly related fix that I need to get in that's related to bumping ECharts to 5.3.3, so I'll see if I can tackle that at the same time.

[villebro]

LGTM with minor nit regarding the storybook (thanks for adding it btw!)

[villebro]

I noticed that the storybook isn't formatting the temporal x-axis correctly:

Could we add an entry for __timestamp to colnames and coltypes which would normally be returned in the chart data payload so the formatting looks correct (2 comes from GenericDataType.TEMPORAL)? I believe this should fix it:

Suggested change

	queriesData={[{ data: negativeNumData }]}
	queriesData={[{ data: negativeNumData, colnames: ['__timestamp'], coltypes: [2] }]}

[justinpark]

Thanks for hotfix! I'll update accordingly

[justinpark]

I have a slightly related fix that I need to get in that's related to bumping ECharts to 5.3.3, so I'll see if I can tackle that at the same time

Since villebro has a plan to fix the negative number position issue, I'll move forward as-is.