Skip to content

Add card height option #3391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/gist.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default async (req, res) => {
border_color,
show_owner,
hide_border,
card_height,
} = req.query;

res.setHeader("Content-Type", "image/svg+xml");
Expand Down Expand Up @@ -74,6 +75,7 @@ export default async (req, res) => {
locale: locale ? locale.toLowerCase() : null,
show_owner: parseBoolean(show_owner),
hide_border: parseBoolean(hide_border),
card_height,
}),
);
} catch (err) {
Expand Down
2 changes: 2 additions & 0 deletions api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default async (req, res) => {
border_color,
rank_icon,
show,
card_height,
} = req.query;
res.setHeader("Content-Type", "image/svg+xml");

Expand Down Expand Up @@ -102,6 +103,7 @@ export default async (req, res) => {
disable_animations: parseBoolean(disable_animations),
rank_icon,
show: showStats,
card_height,
}),
);
} catch (err) {
Expand Down
2 changes: 2 additions & 0 deletions api/pin.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export default async (req, res) => {
locale,
border_radius,
border_color,
card_height,
} = req.query;

res.setHeader("Content-Type", "image/svg+xml");
Expand Down Expand Up @@ -80,6 +81,7 @@ export default async (req, res) => {
border_color,
show_owner: parseBoolean(show_owner),
locale: locale ? locale.toLowerCase() : null,
card_height,
}),
);
} catch (err) {
Expand Down
2 changes: 2 additions & 0 deletions api/top-langs.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export default async (req, res) => {
border_color,
disable_animations,
hide_progress,
card_height,
} = req.query;
res.setHeader("Content-Type", "image/svg+xml");

Expand Down Expand Up @@ -96,6 +97,7 @@ export default async (req, res) => {
locale: locale ? locale.toLowerCase() : null,
disable_animations: parseBoolean(disable_animations),
hide_progress: parseBoolean(hide_progress),
card_height,
}),
);
} catch (err) {
Expand Down
2 changes: 2 additions & 0 deletions api/wakatime.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export default async (req, res) => {
api_domain,
border_radius,
border_color,
card_height,
} = req.query;

res.setHeader("Content-Type", "image/svg+xml");
Expand Down Expand Up @@ -75,6 +76,7 @@ export default async (req, res) => {
locale: locale ? locale.toLowerCase() : null,
layout,
langs_count,
card_height,
}),
);
} catch (err) {
Expand Down
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ You can customize the appearance of all your cards however you wish with URL par
* `cache_seconds` - Sets the cache header manually *(min: 21600, max: 86400)*. Default: `21600 seconds (6 hours)`.
* `locale` - Sets the language in the card, you can check full list of available locales [here](#available-locales). Default: `en`.
* `border_radius` - Corner rounding on the card. Default: `4.5`.
* `card_height` - Card's height. (dynamically)

> [!WARNING]\
> We use caching to decrease the load on our servers (see <https://github.com/anuraghazra/github-readme-stats/issues/1471#issuecomment-1271551425>). Our cards have a default cache of 6 hours (21600 seconds). Also, note that the cache is clamped to a minimum of 6 hours and a maximum of 24 hours. If you want the data on your statistics card to be updated more often you can [deploy your own instance](#deploy-on-your-own) and set [environment variable](#disable-rate-limit-protections) `CACHE_SECONDS` to a value of your choosing.
Expand Down
6 changes: 4 additions & 2 deletions src/cards/gist-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const renderGistCard = (gistData, options = {}) => {
theme,
border_radius,
border_color,
card_height,
show_owner = false,
hide_border = false,
} = options;
Expand All @@ -77,8 +78,9 @@ const renderGistCard = (gistData, options = {}) => {
.join("");

const lineHeight = descriptionLines > 3 ? 12 : 10;
const height =
(descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight;
const height = card_height
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, it would be nice to implement some logic to ensure that the card height is capped at the minimum height needed to display the contents. If I use height=100 the card is messed up 🤔.

image

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure I push a new version do you want the calcul of the min height or you want a fix height :) ?

Copy link
Collaborator

@rickstaa rickstaa Oct 25, 2023

Choose a reason for hiding this comment

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

@DanneelsSophie, I am afraid the minimum heights need to be calculated dynamically for all cards since the parameters will influence the amount of text on the card. I have not yet investigated the best way to do this.

[![Anurag's GitHub stats](https://github-readme-stats.vercel.app/api?username=anuraghazra)](https://github.com/anuraghazra/github-readme-stats)

Anurag's GitHub stats

[![Anurag's GitHub stats](https://github-readme-stats.vercel.app/api?username=anuraghazra&show=reviews,discussions_started,discussion_answered,prs_merged,prs_merged_percentage)](https://github.com/anuraghazra/github-readme-stats)

Anurag's GitHub stats

Copy link
Author

Choose a reason for hiding this comment

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

Do you review the new version ? ( I must test this the calculated height it is min height). I would like this code structure is well for you !

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will review a new version. However, please bear in mind that calculating the minimum required card might be a challenging task. Also, see @qwerty541's comment at #3159 (comment).

Copy link
Author

@DanneelsSophie DanneelsSophie Oct 31, 2023

Choose a reason for hiding this comment

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

I've kept the calculations from before for the heights. I'll try to read the link you sent me (but it doesn't seem as easy as I thought, I'm on vacation until November 6, butI read the comments :) and your indication to finish this task 😊)

Copy link
Collaborator

@rickstaa rickstaa Nov 1, 2023

Choose a reason for hiding this comment

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

I've kept the calculations from before for the heights. I'll try to read the link you sent me (but it doesn't seem as easy as I thought, I'm on vacation until November 6, butI read the comments :) and your indication to finish this task 😊)

@DanneelsSophie, yeah, implementing this is quite intense. We discussed this at #3159 (comment). I think we will close the card_height pull requests for now and migrate to using a description_lines_count parameter instead. Sorry for the inconvenience.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@qwerty541 feel free to close this when you have a description_lines_count pull request 👍🏻.

? card_height
: (descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight;

const totalStars = kFormatter(starsCount);
const totalForks = kFormatter(forksCount);
Expand Down
6 changes: 4 additions & 2 deletions src/cards/repo-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const renderRepoCard = (repo, options = {}) => {
show_owner = false,
theme = "default_repocard",
border_radius,
card_height,
border_color,
locale,
} = options;
Expand All @@ -87,8 +88,9 @@ const renderRepoCard = (repo, options = {}) => {
.map((line) => `<tspan dy="1.2em" x="25">${encodeHTML(line)}</tspan>`)
.join("");

const height =
(descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight;
const height = card_height
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we also need a minimum card_height.

? card_height
: (descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight;

const i18n = new I18n({
locale,
Expand Down
11 changes: 7 additions & 4 deletions src/cards/stats-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ const renderStatsCard = (stats, options = {}) => {
disable_animations = false,
rank_icon = "default",
show = [],
card_height,
} = options;

const lheight = parseInt(String(line_height), 10);
Expand Down Expand Up @@ -391,10 +392,12 @@ const renderStatsCard = (stats, options = {}) => {

// Calculate the card height depending on how many items there are
// but if rank circle is visible clamp the minimum height to `150`
let height = Math.max(
45 + (statItems.length + 1) * lheight,
hide_rank ? 0 : statItems.length ? 150 : 180,
);
let height = card_height
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we also need a minimum card height.

? card_height
: Math.max(
45 + (statItems.length + 1) * lheight,
hide_rank ? 0 : statItems.length ? 150 : 180,
);

// the lower the user's percentile the better
const progress = 100 - rank.percentile;
Expand Down
3 changes: 2 additions & 1 deletion src/cards/top-languages-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ const renderTopLanguages = (topLangs, options = {}) => {
border_radius,
border_color,
disable_animations,
card_height,
} = options;

const i18n = new I18n({
Expand Down Expand Up @@ -805,7 +806,7 @@ const renderTopLanguages = (topLangs, options = {}) => {
customTitle: custom_title,
defaultTitle: i18n.t("langcard.title"),
width,
height,
height: card_height ? card_height : height,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This card also requires the logic for a minimum card height.

border_radius,
colors,
});
Expand Down
1 change: 1 addition & 0 deletions src/cards/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type CommonOptions = {
border_color: string;
locale: string;
hide_border: boolean;
card_height: number;
};

export type StatCardOptions = CommonOptions & {
Expand Down
5 changes: 4 additions & 1 deletion src/cards/wakatime-card.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ const renderWakatimeCard = (stats = {}, options = { hide: [] }) => {
langs_count = languages.length,
border_radius,
border_color,
card_height,
} = options;

const shouldHideLangs = Array.isArray(hide) && hide.length > 0;
Expand Down Expand Up @@ -259,7 +260,9 @@ const renderWakatimeCard = (stats = {}, options = { hide: [] }) => {

// Calculate the card height depending on how many items there are
// but if rank circle is visible clamp the minimum height to `150`
let height = Math.max(45 + (filteredLanguages.length + 1) * lheight, 150);
let height = card_height
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code needs a minimum as well.

? card_height
: Math.max(45 + (filteredLanguages.length + 1) * lheight, 150);

const cssStyles = getStyles({
titleColor,
Expand Down
15 changes: 15 additions & 0 deletions tests/renderGistCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,21 @@ describe("test renderGistCard", () => {
expect(queryByTestId(document.body, "forksCount")).toBeNull();
});

it("should render custom height correctly", () => {
document.body.innerHTML = renderGistCard(
{
...data,
},
{
card_height: 50,
},
);

expect(
document.body.getElementsByTagName("svg")[0].getAttribute("height"),
).toBe("50");
});

it("should render without rounding", () => {
document.body.innerHTML = renderGistCard(data, {
border_radius: "0",
Expand Down
16 changes: 16 additions & 0 deletions tests/renderRepoCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,22 @@ describe("Test renderRepoCard", () => {
expect(queryByTestId(document.body, "badge")).toHaveTextContent("模板");
});

it("should render custom height correctly", () => {
document.body.innerHTML = renderRepoCard(
{
...data_repo.repository,
isTemplate: true,
},
{
card_height: 75,
},
);

expect(
document.body.getElementsByTagName("svg")[0].getAttribute("height"),
).toBe("75");
});

it("should render without rounding", () => {
document.body.innerHTML = renderRepoCard(data_repo.repository, {
border_radius: "0",
Expand Down
10 changes: 10 additions & 0 deletions tests/renderStatsCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,16 @@ describe("Test renderStatsCard", () => {
).toBe("287");
});

it("should render custom height correctly", () => {
document.body.innerHTML = renderStatsCard(stats, {
card_height: 50,
});

expect(
document.body.getElementsByTagName("svg")[0].getAttribute("height"),
).toBe("50");
});

it("should render translations", () => {
document.body.innerHTML = renderStatsCard(stats, { locale: "cn" });
expect(document.getElementsByClassName("header")[0].textContent).toBe(
Expand Down
13 changes: 13 additions & 0 deletions tests/renderTopLanguagesCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -848,4 +848,17 @@ describe("Test renderTopLanguages", () => {
"No languages data.",
);
});

it("should render custom height correctly", () => {
document.body.innerHTML = renderTopLanguages(
{},
{
card_height: 56,
},
);

expect(
document.body.getElementsByTagName("svg")[0].getAttribute("height"),
).toBe("56");
});
});
14 changes: 14 additions & 0 deletions tests/renderWakatimeCard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@ describe("Test Render Wakatime Card", () => {
);
});

it("should render custom height correctly", () => {
document.body.innerHTML = renderWakatimeCard(
{
...wakaTimeData.data,
languages: undefined,
},
{ card_height: 42 },
);

expect(
document.body.getElementsByTagName("svg")[0].getAttribute("height"),
).toBe("42");
});

it('should show "no coding activity this week" message when using compact layout and there has not been activity', () => {
document.body.innerHTML = renderWakatimeCard(
{
Expand Down