add pie chart layout to language card

[arndom]

Added pie chart layout to language card, with some updates in the test script and readme. I wasn't clear about the hide functionality so I left it out for now Should resolve #1650

EDIT (rickstaa): This should be merged after https://github.com/anuraghazra/github-readme-stats/issues/1046 is merged.

[vercel[bot]]

@arndom is attempting to deploy a commit to the github readme stats Team on Vercel.

A member of the Team first needs to authorize it.

[rickstaa]

Great PR. I think this PR will be a great addition to the GRS ecosystem.

However, we might have to scale it down a bit since the card looks a bit large. I will take a look later.

Added pie chart layout to language card, with some updates in the test script and readme. I wasn't clear about the hide functionality so I left it out for now Should resolve #1650

The hide parameter is not needed with your implementation. For now, could you take a look at why the tests are failing (see https://github.com/anuraghazra/github-readme-stats/actions/runs/3172876819/jobs/5167845593)? I suspect it is because you are not waiting for some asynchronous action.

[arndom]

Thanks. I'll have a look at the failing tests later in the day, and maybe the scaling down as well.

[rickstaa]

Great, thanks again! I think the most important thing is that the card should look nice next to the other cards and doesn't appear too big when the maximum number of languages is used.

Maybe we should limit the max number of languages when the pie chart is used. @anuraghazra, what do you think?

[arndom]

@rickstaa I've resolved the failing tests and scaled down the chart, left the decision of limiting languages up to you

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.54 :warning:

Comparison is base (daa1977) 97.35% compared to head (f1c67a5) 96.82%.

:exclamation: Current head f1c67a5 differs from pull request most recent head e118943. Consider uploading reports for the commit e118943 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2099      +/-   ##
==========================================
- Coverage   97.35%   96.82%   -0.54%     
==========================================
  Files          24       22       -2     
  Lines        4311     4000     -311     
  Branches      393      344      -49     
==========================================
- Hits         4197     3873     -324     
- Misses        112      125      +13     
  Partials        2        2              

Impacted Files	Coverage Δ
src/cards/top-languages-card.js	100.00% <100.00%> (ø)

... and 19 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[rickstaa]

@arndom Feel free to request my review if you have finished applying the requested changes to this PR 👍🏻.

[rickstaa]

@arndom Thanks for the improvements. I will look at this PR tomorrow. Very busy week 😅 .

[arndom]

@rickstaa 😅 no worries...you probably saw the notifications, I was trying to request reviews from anuraghazra and you

[rickstaa]

@arndom I reviewed your PR and added some changes (see 2ffb07f6389f0f8154f2dc78b227999001a9ddcd). Now the PR looks good to me. Thanks again for your contribution 🚀. I added the hacktoberfest-accepted label to the PR, so it should show up as a contribution in your Hacktoberfest overview. Could you please review the changes made in 2ffb07f6389f0f8154f2dc78b227999001a9ddcd to see if you agree?

[rickstaa]

@anuraghazra This PR now looks good to me. You can review and merge it later. There are two things we have to decide on before this can be merged:

• Whether we want to rename the layout to doughnut or keep it as pie.
• Whether we want to limit the number of languages on the Pie layout to 5 instead of 10.

[arndom]

@rickstaa Wow, that's some refactor, makes things easier to follow and the tests are pretty detailed, thanks

[rickstaa]

@anuraghazra This PR now looks good to me. You can review and merge it later. There are two things we have to decide on before this can be merged:

• Whether we want to rename the layout to doughnut or keep it as pie.
• Whether we want to limit the number of languages on the Pie layout to 5 instead of 10.

@anuraghazra I fixed this in f1c67a5. The pie chart is now correctly aligned for 5-10 languages.

[rickstaa]

@rickstaa Wow, that's some refactor, makes things easier to follow and the tests are pretty detailed, thanks

No problem. Thanks again for your pull request. I also added https://github.com/anuraghazra/github-readme-stats/commit/f1c67a5edfcf1fc307c932dbcd8951d301f0efa0 to fix the pie chart align problem when more than 5 languages are used. Feel free to check it out.

[rickstaa]

@anuraghazra If you want another excellent PR to merge, I think this one should be it. One of our most requested features (see https://github.com/anuraghazra/github-readme-stats/issues/1935). :rocket: Only thing that can still be added are animations for this new layout (See https://github.com/anuraghazra/github-readme-stats/issues/1046). 🤔

[rickstaa]

@arndom I just wanted to review your pull request again so that it can be merged. I, however, noticed that:

• It contains conflicts.
• It does not contain animations (see https://github.com/anuraghazra/github-readme-stats/issues/1046).
• The layout should be renamed from pie to donut.

I will merge this pull request if those issues are fixed 👍🏻.

[arndom]

Alright @rickstaa I will make the changes

[rickstaa]

Alright @rickstaa I will make the changes

Thanks, no rush. Please let me know when I have to review 👍🏻.

[arndom]

Hey @rickstaa, I've made the changes

[rickstaa]

Hey @rickstaa, I've made the changes

Great work. Thanks again! Will merge when tests are done 👍🏻.

[rickstaa]

@arndom merged. You can now use this on the main branch 🚀.