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

Allow non-primitive data values #808

Closed
3 tasks done
flash1293 opened this issue Sep 8, 2020 · 9 comments · Fixed by #943
Closed
3 tasks done

Allow non-primitive data values #808

flash1293 opened this issue Sep 8, 2020 · 9 comments · Fixed by #943
Assignees
Labels
enhancement New feature or request :Lens Kibana Lens related issue released Issue released publicly

Comments

@flash1293
Copy link

flash1293 commented Sep 8, 2020

Is your feature request related to a problem? Please describe.
In Kibana values in a chart are not always primitive values like strings or numbers (e.g. when working with ranges). In these cases, the table looks like this (this would be rendered as a bar chart with an ordinal x axis):

x y
{ from: 5, to: 20 } 4
{ from: 20, to: 40 } 7

Of course in this case we have a formatter ready to render a nice string tick label out of the object. One solution would be to map over the data and turn all of the objects into the string representation before passing the data to elastic-charts:

x y
5 < x < 20 4
20 < x < 40 7

However the problem with this solution is that it makes it harder to get back to the original object. This is necessary on interactions - e.g. if the user clicks a bar associated with the first row in the table, we need the orginial object to turn it into a filter. The string representation doesn't help in that case.

Currently, elastic-charts seems to filter out rows containing objects, so nothing is rendered:

String values (works)
https://codesandbox.io/s/sweet-sunset-f1s3i?file=/src/App.tsx

Object values (breaks)
https://codesandbox.io/s/festive-jang-z78p6?file=/src/App.tsx

Describe the solution you'd like
Ideally elastic-charts would simply be able to work with objects as values in the data and treat them as ordinal values. Equality checks could be done using JSON.stringify or a similar way of serializing the object.

Describe alternatives you've considered

Another option would be to make it possible to pass "additional data" into the click handler, maybe like this:

<Chart size={[500, 200]}>
        <Settings
          onElementClick={([[geometry]]) => {
            console.log(geometry.clickHandlerContext[0]); // will be { name: "trousers " }
          }}
        />
        <Axis id="count" title="count" position={Position.Left} />
        <Axis
          id="x"
          title="goods"
          position={Position.Bottom}
          tickFormat={d => `xXx${d}xXx`}
        />
        <BarSeries
          id="bars"
          name="amount"
          xScaleType={ScaleType.Ordinal}
          xAccessor="x"
         clickHandlerContextAccessors={["additional"]}
          yAccessors={["y"]}
          data={[{ x: "trousers", additional: { name: "trousers " }, y: 390, val: 1222 },
            { x: "bags", additional: { name: "bags" }, y: 750, val: 1222 }]}
        />
      </Chart>

It's possible to work around this in "user land" as well, by keeping the original table around and doing a reverse lookup in the click handler to get the object for the clicked string, like this: https://codesandbox.io/s/great-browser-ef00i?file=/src/App.tsx

However it would be nice if it wouldn't be necessary to do this intermediary step, especially because it's possible to run into edge cases.

Additional context
A similar issue in vislib was solved by overriding the toString function of the object: elastic/kibana#62004

We are running into this issue in elastic/kibana#76121

Kibana Cross Issues

Checklist

Delete any items that are not applicable to this feature request.

  • this request is checked against already exist requests
  • every related Kibana issue is listed under Kibana Cross Issues list
  • kibana cross issue tag is associated to the issue if any kibana cross issue is present
@flash1293 flash1293 added the enhancement New feature or request label Sep 8, 2020
@flash1293
Copy link
Author

cc @dej611

@wylieconlon wylieconlon added the :Lens Kibana Lens related issue label Sep 8, 2020
@nickofthyme
Copy link
Collaborator

I agree that this feature could be useful. Any thoughts @markov00?

@markov00
Copy link
Member

@nickofthyme definitely.
As first step we can include the original datum to the GeometryValue object passed to the clickHandler. This will give the user the ability to get back the datum with the non-stringified version of the x value if the user, instead of remapping the same object column, adds a new column with the stringified version of the x value and use that on the xAccessor.

@flash1293
Copy link
Author

Sounds even better than clickHandlerContextAccessors @markov00 !

@flash1293
Copy link
Author

@nickofthyme is there still appetite to implement the original proposal? For my use case the exposed datum in the handler callback is fine, so I wouldn't object closing this.

@nickofthyme
Copy link
Collaborator

@flash1293 If you are referring to adding some type of clickHandlerContextAccessors no I think we are going to go with the functional accessor approach instead, see #838. But with regards to the event handler callback, we now include the raw datum which would suit your use case although slightly inconvenient.

If you want to argue that this approach is necessary I'm all ear. 🦻

@flash1293
Copy link
Author

@nickofthyme thanks for the explanation, I’m totally fine with the event handler callback solution.

I was referring to the general ability to have objects in the table, but the issue you linked seems like a nice way to handle that (missed it originally, that’s why I asked).

@nickofthyme
Copy link
Collaborator

@flash1293 Ok perfect. Just to be clear, the current version of elastic-charts allows the functional accessor for the xAccessor value, we just need to allow it on the y and others.

@markov00
Copy link
Member

markov00 commented Dec 9, 2020

🎉 This issue has been resolved in version 24.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request :Lens Kibana Lens related issue released Issue released publicly
Projects
None yet
4 participants