[docs] Fix multiple console.error messages on charts docs
#14554
Conversation
JCQuintas
commented
Sep 9, 2024
JCQuintas
commented
- I have followed (at least) the PR section of the contributing guide.
JCQuintas
added
docs
|
Deploy preview: https://deploy-preview-14554--material-ui-x.netlify.app/ |
CodSpeed Performance ReportMerging #14554 will not alter performanceComparing Summary
|
| @@ -262,6 +262,6 @@ function getGaussianSeriesData(mean, stdev = [0.5, 0.5], N = 50) { | |||
| Math.cos(2.0 * Math.PI * chance.floating({ min: 0, max: 0.99 })) * | |||
| stdev[1] + | |||
| mean[1]; | |||
| return { x, y, z: x + y, id: i }; | |||
There was a problem hiding this comment.
Why adding the mean.join(',') ?
If using indexes causes key to be inconsistent, we could add the seriesId in the key than striking the demo.
There was a problem hiding this comment.
Why adding the mean.join(',') ?
Was a quickfix for having unique ids, the index is already there, adding the mean, which is unique for every call of this function, should make the ids/keys unique overall as well.
Keep in mind these are not in multiple series, they are in a single series, hence why the key=id=i was being an issue
If using indexes causes key to be inconsistent, we could add the seriesId in the key than striking the demo.
Tbh I have no idea why we need the id, I suppose it is to prevent duplicate keys? It feels unnecessary though, like we could use an internal id instead of requiring it, but if the user provides it, we should use what they provide.
There was a problem hiding this comment.
Ok, I did not see that the function was called multiple time to geenrate the data array. I thaugh it it was called once per series
data: [
...getGaussianSeriesData([-1, -1]),
...getGaussianSeriesData([-1, 1]),
...getGaussianSeriesData([1, 1]),
...getGaussianSeriesData([1, -1]),
]
Tbh I have no idea why we need the id,
For sure we could have a default id generation. The only issue is when user start to modify their data. In such case if they do not provide ids by themself, we have no way to properly define key attribute
| @@ -177,7 +177,7 @@ export default function BasicScatter() { | |||
| }, | |||
| { | |||
| label: 'Series B', | |||
| data: data.map((v) => ({ x: v.x1, y: v.y2, id: v.id })), | |||
| data: data.map((v) => ({ x: v.x1, y: v.y2, id: `${v.id}b` })), | |||
There was a problem hiding this comment.
I've two questions
- Is something preventing us to use number as an item id? I assume user with an autoincrement id will have a similar issue
- Is their a specific reason for adding a
bat the end?
There was a problem hiding this comment.
Is something preventing us to use number as an item id?
Probably not, we would just have to ensure the numbers don't overlap, like 1000 + v.id
Is their a specific reason for adding a b at the end?
I just went with the simples way of making this work,
Series B => {id}b
There was a problem hiding this comment.
I do not manage to reproduce the error. I reverted the change on localhost, and I see not message in the console while playing with this demo.
I assume this one can be reverted
There was a problem hiding this comment.
🤔 true, I suppose I got confused with the logs while doing the other scatter issue 🙃
