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

Better tooltip for ranged bar charts #3507

Closed
filipfloden opened this issue Nov 2, 2023 · 5 comments
Closed

Better tooltip for ranged bar charts #3507

filipfloden opened this issue Nov 2, 2023 · 5 comments

Comments

@filipfloden
Copy link

filipfloden commented Nov 2, 2023

Description

I think ranged bar chart tooltip should be changed to the same way #3495 (comment) has been implemented.
Instead of calling tooltip.format.value twice (once with start value and once with end value) it should be called once with value as an array [start, end].

@netil
Copy link
Member

netil commented Nov 7, 2023

it should be called once with value as an array [start, end].

It isn't called twice. It will be called once with array data.
When you add console.log on data.labels.format, it will looks like called multiple times with single number value, because it is the process where determining text length using format function. It can't avoid not calling it, because label text can be modified using it.

@netil netil closed this as completed Nov 7, 2023
@netil netil added the wontfix label Nov 7, 2023
@bj00rn
Copy link

bj00rn commented Nov 7, 2023

it should be called once with value as an array [start, end].

It isn't called twice. It will be called once with array data. When you add console.log on data.labels.format, it will looks like called multiple times with single number value, because it is the process where determining text length using format function. It can't avoid not calling it, because label text can be modified using it.

Here is the line i'm thinking about for tooltip value

value = `${valueFormat(start, undefined, id, index)} ~ ${valueFormat(end, undefined, id, index)}`;

It would be nice if valueFormatter is called with [start, end] in order to show something other than start ~ end.

I would like to show the absolute value for each bar to acheive something like a waterfall chart, showing the absolute values for each bar in both datalabels and tooltip.

#3495 solved the same issue for me for data labels

image

@netil
Copy link
Member

netil commented Nov 8, 2023

Hmm... there was a misunderstanding here. I was pointing that not called twice for data.labels.format option, but you was mentioning on tooltip.format.value option, right?

Updating the way that worked, could derive backward compatibility issue. I'm not sure this is a bug or enhancement.
I'll be considering for that. Thanks anyway.

@netil netil reopened this Nov 8, 2023
netil pushed a commit to netil/billboard.js that referenced this issue Nov 8, 2023
- Make format function called once for range(array) data type
- Update on defaultValueFormat to handle array type value also.

Ref naver#3507
@netil netil closed this as completed in c0f445e Nov 8, 2023
github-actions bot pushed a commit that referenced this issue Nov 8, 2023
## [3.10.3](3.10.2...3.10.3) (2023-11-08)

### Bug Fixes

* **axis:** Fix x axis autorotate option applies ([e45eaf7](e45eaf7)), closes [#3433](#3433) [#3499](#3499)
* **labels:** Fix data.labels rendering for ranged data ([a8ebdef](a8ebdef)), closes [#3495](#3495)
* **option:** Fix rotated top padding  ([048c4e2](048c4e2)), closes [#3433](#3433)
* **tooltip:** Fix tooltip.format.value arg for bar range data ([c0f445e](c0f445e)), closes [#3507](#3507)
@bj00rn
Copy link

bj00rn commented Nov 8, 2023

Hmm... there was a misunderstanding here. I was pointing that not called twice for data.labels.format option, but you was mentioning on tooltip.format.value option, right?

Updating the way that worked, could derive backward compatibility issue. I'm not sure this is a bug or enhancement. I'll be considering for that. Thanks anyway.

Agree, in the case of rangedBarChart it makes sense to have tooltip format return whatever based on [start, end], but I guess changing behaviour would/should affect other ranged chart types as well.

@netil
Copy link
Member

netil commented Nov 8, 2023

well the update only affect for bar ranged type. It could affect some compatibility issue on that type, but will not for other types.

netil pushed a commit to netil/billboard.js that referenced this issue Nov 14, 2023
* **axis:** Fix x axis autorotate option applies ([e45eaf7](naver@e45eaf7)), closes [naver#3433](naver#3433) [naver#3499](naver#3499)
* **labels:** Fix data.labels rendering for ranged data ([a8ebdef](naver@a8ebdef)), closes [naver#3495](naver#3495)
* **option:** Fix rotated top padding  ([048c4e2](naver@048c4e2)), closes [naver#3433](naver#3433)
* **tooltip:** Fix tooltip.format.value arg for bar range data ([c0f445e](naver@c0f445e)), closes [naver#3507](naver#3507)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants