Resolved Same Key Rendering Issue to Namespace ScatterPoints with parent key

[TimonSotiropoulos]

As above, this update should add the parent key to the mapped children points of a scatter component to namespace the points specific to a Scatter rendering which should allow similar data to be displayed correctly on the map

Description

• Added the key of the parent Scatter into the rendered Layer wrapped for the Scatter Points

Related Issue

#4004

Motivation and Context

This change will allow a ScatterChart to render multiple Scatter components built off the same dataset (currently it will throw key matching errors).

How Has This Been Tested?

This has been tested locally but more testing will most likely be required

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• I have added a storybook story or extended an existing story to show my changes

[ckifer]

Will have to think about this a little bit more and ensure this covers most/all cases:

As for your issue in the thread, does this resolve your issue?
https://recharts.org/en-US/examples/ThreeDimScatterChart

Or is that what you mean by "without doing some weird transformations on the data"

I'd imagine adding a z attribute to your data wouldn't be that difficult? I'm not sure what is "weird" about that

[TimonSotiropoulos]

@ckifer The issue is that the dataset is quite large so I was trying to avoid having to run a map on data for every single render (by adding a z attribute to it).

I have made that change and it is working, but now I am having an issue where I can't set the render order of that data array. Reordering the data elements (eg popping the ones I want on top out of the array and adding them back in) is causing the same rendering issue as before. Is there a way to set the zIndex on a Cell component? Or some way to configure the render order for specific data points?

[ckifer]

Yeah makes sense, just checking if that was or wasn't an easy fix for you.

What do you mean by render order, you mean which Scatter shows on top? That is dependent on the order of your Scatter components in your code. SVGs don't support a zIndex property so we can't realistically support that in Cell, as they rely on render order

I.e.

<Scatter />
// this one will show since it was rendered last
<Scatter />

[TimonSotiropoulos]

Yeah that was the problem (there is some talk of being able to order elements by the z value of the actual svg element but that didn't seem to work -> Stack Overflow Link, that seems to be browser related.

As you said the data is rendered in the order it is provided in the array. I was trying to change that order by popping elements and manually reordering them (again to save looping through the entire array again) but that was throwing the same error about multiple items with the same key.

I have just ended up doing a basic sorting on my data through the new z axis I have added which fixes my rendering order issue. So its working now but yeah, I am having to double loop through my array (one to map the zAxis and once to reorder) which is not ideal but at least it is a solution I can live with for now.

Thanks for your help =D

[ckifer]

Yep, nothing we can do about the SVG spec and how the browser interprets it.

Sure, just trying to unblock you while you're waiting. I want to not throw this key error, but I want to make sure the fix is correct which takes more time than replying here :).

Appreciate the responses and understanding!

[ckifer]

I can't actually reproduce this key error locally @TimonSotiropoulos

Would you be able to modify that sandbox in a way that throws the key error? There must be something I'm missing

[ckifer]

Any updates to being able to reproduce this @TimonSotiropoulos ?

[ckifer]

Going to close this until we get a response

[TimonSotiropoulos]

hey @ckifer sorry about the slow reply, been off sick this week.

Here is the modified sandbox doing exactly what I was doing -> Sandbox

It doesn't seem to be throwing any issues there so I guess I will just keep an eye on it and if the error pops up again I will try too recreate it then.

Thanks!

[ckifer]

Hope you feel better!

Thanks! Yeah just feel free to comment back here and I'll see it once we can reproduce. I'll keep an eye out as well