Fix #1170: Allow customizable width for GitHub Stats Card #1334

postatum · 2021-09-28T07:51:22Z

Fixes #1170 by allowing customizable width for Github Stats Card.

To avoid things overlaping each other, Stats Card has minimum widths which are applied when provided custom_width is less than certain values.

vercel · 2021-09-28T07:51:26Z

@postatum 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 · 2021-11-04T09:53:37Z

@postatum Thanks for your pull request. I quickly reviewed your code and it looks good to me. It however looks that the current code doesn't take into account the show_icons argument (see last example below). You might have to reload several times to see all the stats since Github will likely timeout trying to request so many stats at the same time.

hide_rank=true

Test without card_width

Test with to small card_width (i.e 50)

This card width is 220 px smaller than the minimum card width of 270 px.

Test with card_width 300

This card width is 30 px bigger than the minimum card width of 270.

Test with card_width 600

hide_rank=true, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

This card width is 220 px smaller than the minimum card width of 270 px.

Test with card_width 300

This card width is 30 px bigger than the minimum card width of 270.

Test with card_width 600

hide_rank=false

Test without card_width

Test with to small card_width (i.e 50)

This card width is 220 px smaller than the minimum card width of 270 px.

Test with card_width 300

This card width is 30 px bigger than the minimum card width of 270.

Test with card_width 600

hide_rank=false, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

This card width is 220 px smaller than the minimum card width of 270 px.

Test with card_width 300

This card width is 30 px bigger than the minimum card width of 270.

Test with card_width 600

rickstaa · 2021-11-04T10:11:09Z

@postatum, please merge postatum#1 to fix the issues described above. It also trims trialling whitespace, but I can remove that if you want. After this pull request has been merged I will forward it to anuraghazra for review.

hide_rank=false, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

This card width is 220 px smaller than the minimum card width of 270 px.

Test with card_width 300

This card width is 30 px bigger than the minimum card width of 270.

Test with card_width 600

src/cards/stats-card.js

anuraghazra · 2021-11-04T13:37:07Z

The rank circle looks like shifted way too much to the right on the new updates

Left: THIS PR https://github-readme-stats-git-testwidthfix-rickstaa.vercel.app/api?username=anuraghazra&show_icons=true&hide_rank=false

Right: CURRENT https://github-readme-stats.vercel.app/api?username=anuraghazra&show_icons=true&hide_rank=false

This commit fixes a padding problem that was introduced in f9c0e0b. In the new code, the padding around the rank circle will be 50 when the stats card is bigger than 450. When it is smaller than 450 the left and right padding will shrink equally.

rickstaa · 2021-11-04T17:11:31Z

@anuraghazra You are right I missed that. This seems to be caused by the following code that was added by @postatum:

github-readme-stats/src/cards/stats-card.js

Line 230 in dc78bad

transform="translate(${width - 50}, ${height / 2 - 50})">

I added a fix for this to my pull request (see postatum@34d6423).

rickstaa · 2021-11-04T17:19:31Z

Left: THIS PR https://github-readme-stats-git-testwidthfix2-rickstaa.vercel.app/api?username=anuraghazra&show_icons=true&hide_rank=false
Right: CURRENT https://github-readme-stats.vercel.app/api?username=anuraghazra&show_icons=true&hide_rank=false

Visual tests

hide_rank=false

Test without card_width

Without the card, width specified a card width of 495 will be used.

Test with to small card_width (i.e 50)

This card width is 290 px smaller than the minimum card width of 340 px.

Test with card_width 400

This card width is 60 px bigger smaller than the minimum card width of 340.

Test with card_width 600

hide_rank=false, show_icons=true

Test without card_width

Without the card, width specified a card width of 495 will be used.

Test with to small card_width (i.e 50)

This card width is 290 px smaller than the minimum card width of 340 px.

Test with card_width 400

This card width is 60 px bigger than the minimum card width of 340.

Test with card_width 600

anuraghazra · 2021-11-06T13:29:52Z

@postatum can you merge in the changes done by @rickstaa?

Also @rickstaa it's better to keep the PR waterfall minimal, try to code review the PR first instead of committing to the branch of Artem.

rickstaa · 2021-11-06T22:48:10Z

@anuraghazra No problem, I will do that next time. I created the pull request to speed up the development process since it solves one of my problems.

…ation fix: add icon width to stats-card min width calculation

postatum · 2021-11-08T05:47:36Z

Hi guys!

Thanks for reviewing this PR. Sorry, I didn't have time to merge your PR. I did it just now.

rickstaa · 2021-11-08T11:41:52Z

@postatum No problem. Thanks again for creating this pull request!

tests/renderStatsCard.test.js

src/cards/stats-card.js

douglasg14b · 2022-05-01T03:43:34Z

Is this blocked by code review mostly?

rickstaa · 2022-05-01T19:18:11Z

@douglasg14b Thanks for your interest in this feature. I already reviewed this PR some time ago. I, however, have to check it one last time. There are currently three things preventing this PR from being merged:

Fix #1170: Allow customizable width for GitHub Stats Card #1334 (comment): A test for the show_icons needs to be added.
I did to receive a reaction yet from @anuraghazra and @postatum about Fix #1170: Allow customizable width for GitHub Stats Card #1334 (comment). I am happy to implement this change, but some feedback would be excellent.
I need to rebase this on top of the main branch. This needs to be done carefully since fix(style): centering rank text in the circle #1628 changed the code in this PR.

After these points are solved, this PR can be merged. However, I am very short on time since I'm in the final stage of my master's thesis. Therefore, I cannot make a promise when I have time to look at this PR again.

codecov · 2022-09-16T10:19:57Z

Codecov Report

Base: 95.07% // Head: 95.13% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (61e287b) compared to base (192170c).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 61e287b differs from pull request most recent head 247d1a3. Consider uploading reports for the commit 247d1a3 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1334      +/-   ##
==========================================
+ Coverage   95.07%   95.13%   +0.06%     
==========================================
  Files          22       22              
  Lines         771      781      +10     
  Branches      208      214       +6     
==========================================
+ Hits          733      743      +10     
  Misses         33       33              
  Partials        5        5

Impacted Files	Coverage Δ
api/index.js	`90.47% <ø> (ø)`
src/cards/stats-card.js	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

This commit makes sure we also test the stats card width for the case that the 'show_icons' option is enabled.

rickstaa · 2022-09-16T10:52:11Z

Just merged into the master. Sorry for the delay. @postatum Thanks a lot for implementing this feature 🚀.

…nuraghazra#1334) * Change default stats card width with hide rank * Add tests for stats card with card_width * Add card_width Stats Card description to readme * fix: add icon width to stats-card min width calculation * fix: fixes rank circle padding problem This commit fixes a padding problem that was introduced in f9c0e0b. In the new code, the padding around the rank circle will be 50 when the stats card is bigger than 450. When it is smaller than 450 the left and right padding will shrink equally. * style: run prettier * tests: add extra stats 'card_width' tests This commit makes sure we also test the stats card width for the case that the 'show_icons' option is enabled. * style: run prettier Co-authored-by: rickstaa <rick.staa@outlook.com>

postatum added 3 commits September 28, 2021 10:42

Change default stats card width with hide rank

f9c0e0b

Add tests for stats card with card_width

cc28273

Add card_width Stats Card description to readme

dc78bad

rickstaa mentioned this pull request Nov 4, 2021

Add card_width for stats Card repo cards #1170

Closed

rickstaa requested changes Nov 4, 2021

View reviewed changes

src/cards/stats-card.js Outdated Show resolved Hide resolved

src/cards/stats-card.js Outdated Show resolved Hide resolved

rickstaa added 2 commits November 4, 2021 17:59

fix: add icon width to stats-card min width calculation

f642627

Merge pull request #1 from rickstaa/include_icons_in_min_width_calcul…

d8b5c9d

…ation fix: add icon width to stats-card min width calculation

rickstaa requested changes Nov 8, 2021

View reviewed changes

tests/renderStatsCard.test.js Outdated Show resolved Hide resolved

rickstaa requested changes Nov 8, 2021

View reviewed changes

src/cards/stats-card.js Show resolved Hide resolved

rickstaa mentioned this pull request Nov 18, 2021

card_width for Stats Card and Repo Card #1051

Closed

rickstaa mentioned this pull request Dec 4, 2021

Develop/master developStorm/github-readme-stats#1

Merged

rickstaa added the enhancement New feature or request. label May 3, 2022

This was referenced Aug 12, 2022

Parameters / option to alter the GitHub Stats card's size #1939

Closed

Language card card width should not allow cards smaller than the title #1945

Closed

rickstaa added 2 commits September 16, 2022 11:35

Merge branch 'master' into pr/1334

a9c7fba

style: run prettier

61e287b

rickstaa added 2 commits September 16, 2022 12:46

tests: add extra stats 'card_width' tests

bab00bf

This commit makes sure we also test the stats card width for the case that the 'show_icons' option is enabled.

style: run prettier

247d1a3

rickstaa merged commit b0bb994 into anuraghazra:master Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1170: Allow customizable width for GitHub Stats Card #1334

Fix #1170: Allow customizable width for GitHub Stats Card #1334

postatum commented Sep 28, 2021

vercel bot commented Sep 28, 2021

rickstaa commented Nov 4, 2021

rickstaa commented Nov 4, 2021 •

edited

Loading

anuraghazra commented Nov 4, 2021 •

edited

Loading

rickstaa commented Nov 4, 2021 •

edited

Loading

rickstaa commented Nov 4, 2021 •

edited

Loading

anuraghazra commented Nov 6, 2021

rickstaa commented Nov 6, 2021

postatum commented Nov 8, 2021

rickstaa commented Nov 8, 2021

douglasg14b commented May 1, 2022

rickstaa commented May 1, 2022 •

edited

Loading

codecov bot commented Sep 16, 2022 •

edited

Loading

rickstaa commented Sep 16, 2022 •

edited

Loading

Fix #1170: Allow customizable width for GitHub Stats Card #1334

Fix #1170: Allow customizable width for GitHub Stats Card #1334

Conversation

postatum commented Sep 28, 2021

vercel bot commented Sep 28, 2021

rickstaa commented Nov 4, 2021

hide_rank=true

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 300

Test with card_width 600

hide_rank=true, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 300

Test with card_width 600

hide_rank=false

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 300

Test with card_width 600

hide_rank=false, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 300

Test with card_width 600

rickstaa commented Nov 4, 2021 • edited Loading

hide_rank=false, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 300

Test with card_width 600

anuraghazra commented Nov 4, 2021 • edited Loading

rickstaa commented Nov 4, 2021 • edited Loading

rickstaa commented Nov 4, 2021 • edited Loading

Visual tests

hide_rank=false

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 400

Test with card_width 600

hide_rank=false, show_icons=true

Test without card_width

Test with to small card_width (i.e 50)

Test with card_width 400

Test with card_width 600

anuraghazra commented Nov 6, 2021

rickstaa commented Nov 6, 2021

postatum commented Nov 8, 2021

rickstaa commented Nov 8, 2021

douglasg14b commented May 1, 2022

rickstaa commented May 1, 2022 • edited Loading

codecov bot commented Sep 16, 2022 • edited Loading

Codecov Report

rickstaa commented Sep 16, 2022 • edited Loading

rickstaa commented Nov 4, 2021 •

edited

Loading

anuraghazra commented Nov 4, 2021 •

edited

Loading

rickstaa commented Nov 4, 2021 •

edited

Loading

rickstaa commented Nov 4, 2021 •

edited

Loading

rickstaa commented May 1, 2022 •

edited

Loading

codecov bot commented Sep 16, 2022 •

edited

Loading

rickstaa commented Sep 16, 2022 •

edited

Loading