apache / echarts

Apache ECharts is a powerful, interactive charting and data visualization library for browser
https://echarts.apache.org
Apache License 2.0
60.28k stars 19.61k forks source link

[Bug] Incorrect data for aria-label #20063

Open semla opened 3 months ago

semla commented 3 months ago

Version

5.5.0

Link to Minimal Reproduction

https://echarts.apache.org/examples/en/editor.html?c=candlestick-simple&code=PYBwLglsB2AEC8sDeAoWsCGAnCGBcSAptBgEYA2hAJnmFgK6EC-ANGrAB4CCHEAznmTt0VDGHywA2gHIATAAYAjAHYAtIvmrZAFmktYcpWo1aArHoMKV6zbIBsFw9ZOzl0gLrtW7AJ49-gkje6HyEOIQCUsJC6LGwYD4ghILSAMYY0FSUfJCpANZ60SJiEpJFsZIK-gDM2voaNQAc7mxxcZLa8jWmNV2wpvIt5eiS1YpNNdX62tpDbRXVjfU9_dOynvMbsUzsnkwA3EA

Steps to Reproduce

Look at the aria-label in the link.

Current Behavior

aria-label="This is a chart with type Candlestick.The data is as follows: the data for 2017-10-24 is 0, 20, 34, 10, 38, the data for 2017-10-25 is 1, 40, 35, 30, 50, the data for 2017-10-26 is 2, 31, 38, 33, 44, the data for 2017-10-27 is 3, 38, 15, 5, 42. "

an extra number is inserted before the correct data

Expected Behavior

aria-label="This is a chart with type Candlestick.The data is as follows: the data for 2017-10-24 is 20, 34, 10, 38, the data for 2017-10-25 is 40, 35, 30, 50, the data for 2017-10-26 is 31, 38, 33, 44, the data for 2017-10-27 is 38, 15, 5, 42. "

Environment

- OS:
- Browser:
- Framework:

Any additional comments?

Ran into it when working with 20038 Here is another example where the data for aria-label seems more confusing

helgasoft commented 3 months ago

Your code just enables aria without setting the label (?!) Demo works.

<div id="chart-container" _echarts_instance_="ec_1719277587102" 
 aria-label="This is a chart with type Candlestick.The data is as follows: the data for 2017-10-24 is 0, 20, 34, 10, 38, the data for 2017-10-25 is 1, 40, 35, 30, 50, the data for 2017-10-26 is 2, 31, 38, 33, 44, the data for 2017-10-27 is 3, 38, 15, 5, 42. " 
 style="user-select: none; -webkit-tap-highlight-color: rgba(0, 0, 0, 0);"><div style="position: relative; width: 501px; height: 575px; padding: 0px; margin: 0px; border-width: 0px; cursor: default;"><canvas data-zr-dom-id="zr_0" width="551" height="632" style="position: absolute; left: 0px; top: 0px; width: 501px; height: 575px; user-select: none; -webkit-tap-highlight-color: rgba(0, 0, 0, 0); padding: 0px; margin: 0px; border-width: 0px;"></canvas></div>
</div>

📌 please close issue if problem solved.

semla commented 3 months ago

Your code just enables aria without setting the label (?!)

I am pretty sure the aria-label is generated if not set? What I was referring to was the data generated that seems incorrect.

semla commented 3 months ago

Seems like on some charts like bar, line, candlestick (not exhaustive list) each data item has an extra data point. It is the first one and seems like an index ("the data for Mon is 0, 1, the data for Tue is 1, -2,.. etc)

Here is a "naive" theory that maybe is useful? I took a quick look at the code for the dataview in the toolbox, and it looks like there is a separation of chart-types: https://github.com/apache/echarts/blob/a6d046302fb6ed9baae920837b0b7bdae3d715c6/src/component/toolbox/feature/DataView.ts#L64-L66 In aria.ts it looks like it just returns the values from getData regardless of the type of chart: https://github.com/apache/echarts/blob/a6d046302fb6ed9baae920837b0b7bdae3d715c6/src/visual/aria.ts#L207

semla commented 3 months ago

@Ovilia Are you sure it is not a bug?

Ovilia commented 3 months ago

Although I won't say this is the best thing we could do since the code in ARIA may be somewhat straightforward, I don't think this should be regarded as a bug. You may make a pull request to improve it.

20038 tries to fix this by providing an option to let developers hide certain dimensions, while you could also change the default dimensions to display, like using the second instead of all in the case of candlestick series. Both plans solves the problem to some extent and it's either way works.

semla commented 3 months ago

Ok, yes https://github.com/apache/echarts/pull/20038 should be usable to avoid the extra index data.

Here the data in aria-label is also repeated.

You may make a pull request to improve it.

Not sure I know enough but maybe I'll give it a shot. Is it a good idea to look at the code for the dataview ? iiuc that code deals differently with cartesian2D charts like lines, bar, candlestick - which seems to be the ones that are problematic