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

feat(Line) Allow pointLabel AccessorFunc to return the full point info #1981

Closed
wants to merge 1 commit into from

Conversation

Binb1
Copy link

@Binb1 Binb1 commented Apr 28, 2022

General request

Hi,

I noticed that the label property of the @bar chart allowed to get the full point info (id, value, ...).
It seems that the AccessorFunc of the @line chart only pass down the data.

I think that enhancing this function to support the full Point type would allow a better customisation at the data point level.

My proposition is to add the point: Point directly to the function in order to avoid any breaking changes, but if you think that updating the datum: Point['data'] to datum: Point is more suitable, I can update my PR to match this quickly.

Case details

I'm requesting this change because in my case I need to format the value at a data point level.
Technically, every data point could have a different formatting in my charts, and without getting the id behind the value (contained in the Point but not the Point['data']), I have no way to know which value is linked to which formatting if I have more than 1 time the same value.

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 17c99d7:

Sandbox Source
nivo Configuration

@Binb1 Binb1 changed the title @Line Allow pointLabel AccessorFunc to return the full point info feat(Line) Allow pointLabel AccessorFunc to return the full point info Apr 29, 2022
@stale stale bot added stale and removed stale labels Jul 30, 2022
Repository owner deleted a comment from stale bot Sep 8, 2022
Repository owner deleted a comment from Binb1 Sep 8, 2022
@plouc plouc added the pinned label Sep 8, 2022
@dubzzz
Copy link
Contributor

dubzzz commented Apr 26, 2023

Any chance to have this change merged in one of the future releases?

@dubzzz
Copy link
Contributor

dubzzz commented Mar 8, 2024

@plouc Any blocker for this one? It would help me to have it merged or work on helping merging it one day.

@plouc
Copy link
Owner

plouc commented Mar 11, 2024

@dubzzz, I didn't review the PR yet, but there are some conflicts.

@dubzzz
Copy link
Contributor

dubzzz commented Mar 11, 2024

If you are ok with the change in 17c99d7, I can give it a try and re-open a new PR doing it. Thanks for the quick reply

@plouc
Copy link
Owner

plouc commented Mar 11, 2024

@dubzzz, sure, I think this could be useful to some users, it would be a breaking change, but maybe it would be best to directly pass the point rather than adding a second argument, as data is part of it anyway. It would also be interesting to add tests for this.

@akassaei
Copy link
Contributor

Hello @plouc ,
After synching with @dubzzz on the matter, we decided to create a separate PR for this
You can find it here : #2541
If you have some time it would be nice to have a review, thanks a lot for all the work you put on this 🙏

@dubzzz
Copy link
Contributor

dubzzz commented Mar 28, 2024

Thanks a lot for the new version of the PR @akassaei, looking forward to play with it ❤️

@plouc
Copy link
Owner

plouc commented May 1, 2024

Superseded by #2541.

@plouc plouc closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants