Conversation
Signed-off-by: Albert Kyei <akyei908@gmail.com>
Signed-off-by: Albert Kyei <akyei908@gmail.com>
|
Hi @texodus, could you take a look this PR to let me know if anything is amiss, thanks. |
|
Thanks for submitting this! Although this succeeds for simple cases, it does not work for more complex examples. Luckily the code you wrote is on the right path, it only needs to be generalized further. Here is a config that shows that your fix is not enough. Using the example data from #2575 and the file example, providing more than 1 split overwhelms your fix. Instead of hardcoding one level of split, you need to allow for any number of splits. To apply the config you can use the debug panel provided in the top right of the sidebar. Also, We generally require a unit test for any fix, regardless of complexity. If you could write a unit test that ensures that this functionality works, it would be amazing! Again thanks for your work in fixing this issue, I hope we can get this merged soon! |
texodus
left a comment
There was a problem hiding this comment.
See @sinistersnare's feedback pls
|
@sinistersnare Thanks for the feedback and suggestions. I'll work on it further. |
Signed-off-by: Albert Kyei <akyei908@gmail.com>
Signed-off-by: Albert Kyei <akyei908@gmail.com>
texodus
left a comment
There was a problem hiding this comment.
Thanks for the PR! Looks good!
This PR addresses the bug reported in #2575 and #2254 where tooltip data labels were missing their corresponding values when multiple data values are configured. The issue stemmed from an invalid property used to fetch tooltip data from the
data.rowJSON, which was missing a required “prefix”. This has now been corrected.