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(#85): expose selectedIndexes to tooltip renderer #80

Closed
wants to merge 1 commit into from

Conversation

canastro
Copy link
Contributor

@canastro canastro commented Apr 8, 2022

This is a proposal to #85 and it's a breaking change:

Context

In our app we are using a tooltipRenderer but we need to be able to customize the display of each field on the tooltip based on information we dont have available in the tuples that are provided to Graphic. Therefore we need to get back the "index" of the selected tuple so that I can get the appropriate record from our data structure. Thats the gist of what we need.

To give a bit more context, we're dynamically creating charts in a spreadsheet application with data provided by the user and, as google sheets we want to allow the user to have values with different formatting in each point.

Here is an example where the user defined different number of decimal places per cell via formatting, ie. the user did not introduce 1.000000, the actual value is "1" but the formatting is "use 6 decimal places => 1.000000":

Screen.Recording.2022-04-17.at.08.37.53.mov

Proposal

In the tooltipRenderer return back the "selectedIndexes" to allow the user to find the appropriate record on a more complex datastructure on user-land. For users not using "tooltipRenderer", provide "tooltipFieldFormatter" which receives the selectedIndex for the current tuple being formatted.

Let me know what you think of the overall approach and if there are any alternative ways of doing this.

Thank you.

@entronad
Copy link
Owner

  1. To add the selectedIndexes is a good idea. With it, you can find your original datum, and make usage of it's special attributes which are not in the tuples. There are some conflicts of this PR, so for convenience I will add it myself.
  2. I don't think "tooltipFieldFormatter" is a good idea. It's function are all covered by the renderer, and it's hard to get what this argument really means. (I got the meaning of "field formatter" only after I carefully read the source code.). So, with the scale.formatter for common users, and renderer for senior users, I think the tooltipFieldFormatter is not that necessary.

@canastro
Copy link
Contributor Author

Alright, makes sense. I’ll review the PR and I’ll let you know when it’s ready for further review

@canastro canastro changed the title proposal: support tooltip field formatters feat(#85): expose selectedIndexes to tooltip renderer Apr 27, 2022
@canastro
Copy link
Contributor Author

@entronad I misread the part for convenience I will add it myself. and rebased and simplified this PR.
Feel free to ignore this PR and close it if you're already working on it.

Comment on lines +51 to +55
List<Figure> simpleTooltip({
required Offset anchor,
required List<Tuple> selectedTuples,
required Set<int> selectedIndexes,
}) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't created any example for the selectedIndexes, let me know if you want me to add one.

@entronad
Copy link
Owner

Added in v0.10.0.

@entronad entronad closed this Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants