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
refactor: createThumbnailFromPath
takes size
not maxSize
#37362
Conversation
createThumbnailFromPath
maximum size enforcementcreateThumbnailFromPath
should allow size smaller than maxSize
createThumbnailFromPath
should allow size smaller than maxSize
createThumbnailFromPath
maxSize
enforcement
07c6d7b
to
8fc3ded
Compare
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.
This maxSize
option doesn't make sense. We should replace it with size
in a breaking change.
The native APIs for both Windows and macOS take a size
param -- not maxSize
.
More importantly, neither has API to tell us the preferred of a thumbnail so that we can clamp it. And it makes sense for that API to be missing, because what would the preferred size of a text file thumbnail even mean?
Life will be easier for both us and our users if our API exposes fields that map directly to native API.
History: maxSize
was included in the feature rollout @ #24802
createThumbnailFromPath
maxSize
enforcement8fc3ded
to
27098e1
Compare
27098e1
to
aa135a1
Compare
@ckerr updated to make this a breaking change! |
createThumbnailFromPath
takes size
not maxSize
aa135a1
to
8869aa9
Compare
8869aa9
to
8427281
Compare
8427281
to
2cb262e
Compare
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.
API LGTM
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.
API LGTM
Release Notes Persisted
|
I was unable to backport this PR to "24-x-y" cleanly; |
@VerteDinde has manually backported this PR to "24-x-y", please check out #37796 |
Description of Change
Closes #37340.
The
maxSize
parameter has been changed tosize
to reflect that the size passed in will be the size the thumbnail created. Previously, Windows would not scale the image up if it were smaller thanmaxSize
, andmacOS would always set the size to
maxSize
. Behavior is now the same across platforms.Checklist
npm test
passesRelease Notes
Notes:
nativeImage.createThumbnailFromPath()
now takessize
instead ofmaxSize
.