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.

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

🚀 Feature: Add explicit sizing for README.md logo, if provided #1203

Closed
3 tasks done
JoshuaKGoldberg opened this issue Jan 6, 2024 · 0 comments · Fixed by #1806
Closed
3 tasks done

🚀 Feature: Add explicit sizing for README.md logo, if provided #1203

JoshuaKGoldberg opened this issue Jan 6, 2024 · 0 comments · Fixed by #1806
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request

Comments

@JoshuaKGoldberg
Copy link
Owner

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Overview

This repository makes use of the --logo option from #851 + #1166:

<img align="right" alt="Project logo: the TypeScript blue square with rounded corners, but a plus sign instead of 'TS'" src="./create-typescript-app.png">

But, note how there's no explicit image height or width attribute in there. That means the page won't know how to account for the image's size until it loads in. Kind of a performance issue.

In JoshuaKGoldberg/all-contributors-auto-action#188 I added a logo to the all-contributors-auto-action repository that does have explicit height and width attributes. Let's add in explicit height and width to this template too!

Additional Info

The logo image is created here:

<img align="right" alt="${options.logo.alt}" src="${options.logo.src}">

However, I think this feature shouldn't assume a 128px by 128px image. It should:

  1. Read in the logo as an image file from disk
    • If the image file doesn't exist or can't be read as an image, throw an error
  2. Use Math.min(image size, 128px) for both height and width
    • If either are > 192px, log a warning that it shouldn't be used as a logo
@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request status: accepting prs Please, send a pull request to resolve this! labels Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant