-
-
Notifications
You must be signed in to change notification settings - Fork 21
fix: Small thumbnails not appearing #1335
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
Conversation
pkg.pr.new packages
benchmark commit |
query: 'w=512;1024', | ||
}) as Record<string, [string, string]>, | ||
}) as Record<string, string | [string, string]>, | ||
R.mapKeys(pathToExampleKey), | ||
R.mapValues((value): ThumbnailPair => ({ | ||
small: value[0], | ||
large: value[1], | ||
})), | ||
R.mapValues(( | ||
value, | ||
): ThumbnailPair => (typeof value === 'string' | ||
? { small: value, large: value } | ||
: { small: value[0], large: value[1] }) | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably prefer having a minimum size requirement for the thumbnail size and throw a descriptive error if it's too small. This would guarantee consistent look for the thumbnails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's go 🎉
Also, could we change the base to |
47ddb34
to
405238d
Compare
To my understanding, the query
w=512;1024
only creates two result images if the width of the input image is at least 513, because the only allowed operation by default is downscaling.