Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add card_width variable to the repository card #3368 #3359

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Add card_width variable to the repository card #3368 #3359

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0f2e07e
Update repo-card.js
airwakz Oct 14, 2023
39c75af
Update repo-card.js
airwakz Oct 15, 2023
6348f24
Update top-langs.js
airwakz Oct 15, 2023
3241da6
Update top-languages-card.js
airwakz Oct 15, 2023
3afdb19
Update repo-card.js
airwakz Oct 15, 2023
4bc71ad
Update repo-card.js
airwakz Oct 15, 2023
bf70a72
Update top-langs.js
airwakz Oct 15, 2023
bc645c0
Update pin.js
airwakz Oct 15, 2023
1c0928b
Update top-languages-card.js
airwakz Oct 15, 2023
ab5419b
Update src/cards/repo-card.js
airwakz Oct 16, 2023
0de9a70
Update stats-card.js
airwakz Oct 16, 2023
81a24f5
Update stats-card.js
airwakz Oct 16, 2023
829e1ab
Update repo-card.js
airwakz Oct 16, 2023
5467847
Update top-langs.js
airwakz Oct 16, 2023
7cd57f2
fix: restore 'top-languages-card'
rickstaa Oct 16, 2023
18db53d
Merge branch 'master' into airwakz/master
rickstaa Oct 16, 2023
49b72b8
fix: fix bugs and add tests
rickstaa Oct 16, 2023
49aef2e
Update readme.md
airwakz Oct 16, 2023
fa2a74d
Update readme.md
airwakz Oct 16, 2023
d6816a3
Update readme.md
airwakz Oct 17, 2023
91ba93e
Update repo-card.js
airwakz Oct 18, 2023
4147062
Update readme.md
airwakz Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions api/top-langs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export default async (req, res) => {
hide,
hide_title,
hide_border,
card_width,
card_width, // Add card_width and card_height here
Copy link
Collaborator

Choose a reason for hiding this comment

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

@airwakz can you remove these changes? These changes are not related to the pinned card. You should add your changes to https://github.com/anuraghazra/github-readme-stats/blob/master/api/pin.js.

card_height, // Add card_height here
title_color,
text_color,
bg_color,
Expand Down Expand Up @@ -84,6 +85,7 @@ export default async (req, res) => {
hide_title: parseBoolean(hide_title),
hide_border: parseBoolean(hide_border),
card_width: parseInt(card_width, 10),
card_height: parseInt(card_height, 10),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not do this in this pull request but do it later in another pull request.

hide: parseArray(hide),
title_color,
text_color,
Expand All @@ -104,7 +106,7 @@ export default async (req, res) => {
`max-age=${CONSTANTS.ERROR_CACHE_SECONDS / 2}, s-maxage=${
CONSTANTS.ERROR_CACHE_SECONDS
}, stale-while-revalidate=${CONSTANTS.ONE_DAY}`,
); // Use lower cache period for errors.
); // Use a lower cache period for errors.
return res.send(renderError(err.message, err.secondaryMessage));
}
};
18 changes: 12 additions & 6 deletions src/cards/repo-card.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ const getBadgeSVG = (label, textColor) => `
*
* @param {RepositoryData} repo Repository data.
* @param {Partial<RepoCardOptions>} options Card options.
* @param {number} card_width The width of the card.
* @param {number} card_height The height of the card.
* @returns {string} Repository card SVG object.
*/
const renderRepoCard = (repo, options = {}) => {
const renderRepoCard = (repo, options = {card_width,card_height}) => {
airwakz marked this conversation as resolved.
Show resolved Hide resolved
const {
name,
nameWithOwner,
Expand All @@ -73,6 +75,8 @@ const renderRepoCard = (repo, options = {}) => {
border_radius,
border_color,
locale,
card_width,
card_height,
} = options;

const lineHeight = 10;
Expand All @@ -87,8 +91,7 @@ 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 || (descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a minimum card height to prevent the card height from being too small to display the card's content.

Copy link
Author

Choose a reason for hiding this comment

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

what height should i take

Copy link
Author

Choose a reason for hiding this comment

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

`/**

  • Renders repository card details.
  • @param {RepositoryData} repo Repository data.
  • @param {Partial} options Card options.
  • @param {number} card_width The width of the card.
  • @param {number} card_height The height of the card.
  • @returns {string} Repository card SVG object.
    */
    const renderRepoCard = (repo, options = {}, card_width, card_height) => {
    // ... (existing code)

// Calculate a minimum card height to ensure content is visible
const minimumHeight = 150; // You can adjust this value as needed

// Calculate the final card height
const finalHeight = Math.max(
minimumHeight, // Ensure it doesn't go below the minimum height
(descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight
);

// ... (rest of the function)

const card = new Card({
defaultTitle: header.length > 35 ? ${header.slice(0, 35)}... : header,
titlePrefixIcon: icons.contribs,
width: card_width, // Use the calculated card width
height: card_height || finalHeight, // Use the provided card height or the calculated one
border_radius,
colors,
});

// ... (rest of the function)

return card.render(`
${
isTemplate
? // @ts-ignore
getBadgeSVG(i18n.t("repocard.template"), colors.textColor)
: isArchived
? // @ts-ignore
getBadgeSVG(i18n.t("repocard.archived"), colors.textColor)
: ""
}

<text class="description" x="25" y="-5">
  ${descriptionSvg}
</text>

<g transform="translate(30, ${finalHeight - 75})">
  ${starAndForkCount}
</g>

); };

Copy link
Collaborator

Choose a reason for hiding this comment

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

what height should i take

Judging from a repository with only a 1 line description, this height should be 118.8. You can check this out by going to https://github-readme-stats.vercel.app/api/pin/?username=anuraghazra&repo=github-readme-stats and checking out the height of the card using chromes developer tools.

Copy link
Collaborator

Choose a reason for hiding this comment

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

`/**

  • Renders repository card details.
  • @param {RepositoryData} repo Repository data.
  • @param {Partial} options Card options.
  • @param {number} card_width The width of the card.
  • @param {number} card_height The height of the card.
  • @returns {string} Repository card SVG object.
    */
    const renderRepoCard = (repo, options = {}, card_width, card_height) => {
    // ... (existing code)

// Calculate a minimum card height to ensure content is visible const minimumHeight = 150; // You can adjust this value as needed

// Calculate the final card height const finalHeight = Math.max( minimumHeight, // Ensure it doesn't go below the minimum height (descriptionLines > 1 ? 120 : 110) + descriptionLines * lineHeight );

// ... (rest of the function)

const card = new Card({ defaultTitle: header.length > 35 ? ${header.slice(0, 35)}... : header, titlePrefixIcon: icons.contribs, width: card_width, // Use the calculated card width height: card_height || finalHeight, // Use the provided card height or the calculated one border_radius, colors, });

// ... (rest of the function)

return card.render(` ${ isTemplate ? // @ts-ignore getBadgeSVG(i18n.t("repocard.template"), colors.textColor) : isArchived ? // @ts-ignore getBadgeSVG(i18n.t("repocard.archived"), colors.textColor) : "" }

<text class="description" x="25" y="-5">
  ${descriptionSvg}
</text>

<g transform="translate(30, ${finalHeight - 75})">
  ${starAndForkCount}
</g>

); };

Please apply changes to the pull request. It is hard for me to review pasted code 😅.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You should apply the conditions for your minimum height here.


const i18n = new I18n({
locale,
Expand All @@ -115,13 +118,13 @@ const renderRepoCard = (repo, options = {}) => {
icons.star,
totalStars,
"stargazers",
ICON_SIZE,
ICON_SIZE
);
const svgForks = iconWithLabel(
icons.fork,
totalForks,
"forkcount",
ICON_SIZE,
ICON_SIZE
);

const starAndForkCount = flexLayout({
Expand All @@ -134,10 +137,13 @@ const renderRepoCard = (repo, options = {}) => {
gap: 25,
}).join("");

// Calculate the card width and height based on the provided arguments or defaults
const width = card_width || 400;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need to apply a minimum card width. Similar to is done in the langs and stats cards.


const card = new Card({
defaultTitle: header.length > 35 ? `${header.slice(0, 35)}...` : header,
titlePrefixIcon: icons.contribs,
width: 400,
width,
height,
border_radius,
colors,
Expand Down
6 changes: 4 additions & 2 deletions src/cards/top-languages-card.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,8 @@ const renderTopLanguages = (topLangs, options = {}) => {
layout,
custom_title,
locale,
card_width = DEFAULT_CARD_WIDTH,
rickstaa marked this conversation as resolved.
Show resolved Hide resolved
card_height = calculateNormalLayoutHeight(topLangs.length),
langs_count = getDefaultLanguagesCountByLayout({ layout, hide_progress }),
border_radius,
border_color,
Expand Down Expand Up @@ -804,8 +806,8 @@ const renderTopLanguages = (topLangs, options = {}) => {
const card = new Card({
customTitle: custom_title,
defaultTitle: i18n.t("langcard.title"),
width,
height,
width: card_width,
height: card_height,
border_radius,
colors,
});
Expand Down