Skip to content

[charts] Use real world data for PieChart examples#14297

Merged
JCQuintas merged 7 commits intomui:masterfrom
JCQuintas:use-real-world-pie-data
Sep 2, 2024
Merged

[charts] Use real world data for PieChart examples#14297
JCQuintas merged 7 commits intomui:masterfrom
JCQuintas:use-real-world-pie-data

Conversation

@JCQuintas
Copy link
Copy Markdown
Member

@JCQuintas JCQuintas commented Aug 22, 2024

Pie charts are quite simple charts that can't contain a lot of data, and our examples were geared towards that optimistic view.

Real world usage however, is more complicated.

I've tried using world population datasets (by continent and sub-regions, eg: Europe (continent), Balkans (sub-continent). But that created charts with too many small slices, due to what I guess is population concentration 🙃

My second try was internet browser usage, and landed on Platforms and Operating Systems (by platform) which provided a nice mix of very simple to slightly more complex data.

  • Using data from https://gs.statcounter.com
  • From December 2023
  • Data was manipulated in regards to condensing values that were too small into Other and removing Tablet from platforms

Data points

  • platforms: 2 data points
  • desktopOS: 5
  • mobileOS: 3 (only used in mobileAndDesktopOS)
  • mobileAndDesktopOS: 8

Preview

https://deploy-preview-14297--material-ui-x.netlify.app/x/react-charts/pie/

Side by Side comparison
Old New
screencapture-localhost-3001-x-react-charts-pie-2024-08-23-12_19_10 screencapture-localhost-3001-x-react-charts-pie-2024-08-23-12_17_57

@JCQuintas JCQuintas added docs Improvements or additions to the documentation. scope: charts Changes related to the charts. labels Aug 22, 2024
@JCQuintas JCQuintas self-assigned this Aug 22, 2024
@mui-bot
Copy link
Copy Markdown

mui-bot commented Aug 22, 2024

Deploy preview: https://deploy-preview-14297--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 20486ad

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Aug 22, 2024

CodSpeed Performance Report

Merging #14297 will not alter performance

Comparing JCQuintas:use-real-world-pie-data (20486ad) with master (0c30a6f)

Summary

✅ 3 untouched benchmarks

Copy link
Copy Markdown
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

I like the OS dataset. That's for sure something user will understand :)

Comment thread docs/data/charts/pie/PieClickNoSnap.js Outdated
Comment thread docs/data/charts/pie/PieClickNoSnap.js Outdated
JCQuintas and others added 2 commits August 26, 2024 16:44
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 27, 2024
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@dosubot dosubot Bot added the size:L label Aug 27, 2024
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 27, 2024
Copy link
Copy Markdown
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

Looks super nice. I enjoyed reading back this docs page with the new demo 👍

},
];

const normalize = (v: number, v2: number) => Number.parseFloat(((v * v2) / 100).toFixed(2));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Recently found that formatting method which simplify the management of digits after comma

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat#style

new Intl.NumberFormat('de-DE', { style: 'percent', maximumFractionDigits: 2  }).format(
    number,
  ),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think using the number format would be better in this case, we are clipping the number number so they are easier to read, yes, but they also need to be a numeric value, while Intl.NumberFormat would format it to a user to see, like adding . in the hundreds, eg: 1000 -> 1.000

Copy link
Copy Markdown
Member

@alexfauquette alexfauquette Sep 2, 2024

Choose a reason for hiding this comment

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

What triggered me is the .toFixed(2) before parsing.

You have do

  1. number computation (v * v2) / 100)
  2. transformation to string .toFixed(2)
  3. transformation to number Number.parseFloat()

Whereas you could

  1. have the correct numbers by doing only the (v * v2) / 100) in the normalize
  2. display value as expected with
const valueFormatter = new Intl.NumberFormat('de-DE', { style: 'percent', maximumFractionDigits: 2  }).format;

Intl.NumberFormat would format it to a user to see, like adding . in the hundreds, eg: 1000 -> 1.000

Yes, and with style: 'percent' it will add the percentage sygn, plus some space according to the locale, an manage rounding the values to get the correct number of digits, ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, that could be an options yes.

I don't think it matters much though, since we also have the data as 11.11 in the other dataset, this ensures all datasets have the same precision. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation. scope: charts Changes related to the charts.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants