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

refactor: createThumbnailFromPath takes size not maxSize #37362

Merged
merged 1 commit into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 docs/api/native-image.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,15 @@ Returns `NativeImage`

Creates an empty `NativeImage` instance.

### `nativeImage.createThumbnailFromPath(path, maxSize)` _macOS_ _Windows_
### `nativeImage.createThumbnailFromPath(path, size)` _macOS_ _Windows_

* `path` string - path to a file that we intend to construct a thumbnail out of.
* `maxSize` [Size](structures/size.md) - the maximum width and height (positive numbers) the thumbnail returned can be. The Windows implementation will ignore `maxSize.height` and scale the height according to `maxSize.width`.
* `size` [Size](structures/size.md) - the desired width and height (positive numbers) of the thumbnail.

Returns `Promise<NativeImage>` - fulfilled with the file's thumbnail preview image, which is a [NativeImage](native-image.md).

Note: The Windows implementation will ignore `size.height` and scale the height according to `size.width`.

### `nativeImage.createFromPath(path)`

* `path` string
Expand Down
35 changes: 35 additions & 0 deletions docs/breaking-changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,43 @@
* **Deprecated:** An API was marked as deprecated. The API will continue to function, but will emit a deprecation warning, and will be removed in a future release.
* **Removed:** An API or feature was removed, and is no longer supported by Electron.

## Planned Breaking API Changes (24.0)

Check failure on line 15 in docs/breaking-changes.md

View check run for this annotation

trop / Backportable? - 24-x-y

docs/breaking-changes.md#L15

Patch Conflict
Raw output
++<<<<<<< HEAD
 +## Planned Breaking API Changes (23.0)
++=======
+ ## Planned Breaking API Changes (24.0)
+ 
+ ### API Changed: `nativeImage.createThumbnailFromPath(path, size)`
+ 
+ The `maxSize` parameter has been changed to `size` 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 than `maxSize`, and
+ macOS would always set the size to `maxSize`. Behavior is now the same across platforms.
+ 
+ Updated Behavior:
+ 
+ ```js
+ // a 128x128 image.
+ const imagePath = path.join('path', 'to', 'capybara.png')
+ 
+ // Scaling up a smaller image.
+ const upSize = { width: 256, height: 256 }
+ nativeImage.createThumbnailFromPath(imagePath, upSize).then(result => {
+   console.log(result.getSize()) // { width: 256, height: 256 }
+ })
+ 
+ // Scaling down a larger image.
+ const downSize = { width: 64, height: 64 }
+ nativeImage.createThumbnailFromPath(imagePath, downSize).then(result => {
+   console.log(result.getSize()) // { width: 64, height: 64 }
+ })
+ ```
+ 
+ Previous Behavior (on Windows):
+ 
+ ```js
+ // a 128x128 image
+ const imagePath = path.join('path', 'to', 'capybara.png')
+ const size = { width: 256, height: 256 }
+ nativeImage.createThumbnailFromPath(imagePath, size).then(result => {
+   console.log(result.getSize()) // { width: 128, height: 128 }
+ })
+ ```
+ 
+ ### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`
+ 
+ `BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the
+ `BrowserWindow.setWindowButtonPosition(position)` API should be used instead
+ which accepts `null` instead of `{ x: 0, y: 0 }` to reset the position to
+ system default.
+ 
+ ```js
+ // Removed in Electron 24
+ win.setTrafficLightPosition({ x: 10, y: 10 })
+ win.setTrafficLightPosition({ x: 0, y: 0 })
+ 
+ // Replace with
+ win.setWindowButtonPosition({ x: 10, y: 10 })
+ win.setWindowButtonPosition(null)
+ ```
+ 
+ ### Deprecated: `BrowserWindow.getTrafficLightPosition()`
++>>>>>>> refactor: createThumbnailFromPath takes size not maxSize

Check failure on line 15 in docs/breaking-changes.md

View check run for this annotation

trop / Backportable? - 24-x-y

docs/breaking-changes.md#L15

Patch Conflict
Raw output
++<<<<<<< HEAD
 +## Planned Breaking API Changes (23.0)
++=======
+ ## Planned Breaking API Changes (24.0)
+ 
+ ### API Changed: `nativeImage.createThumbnailFromPath(path, size)`
+ 
+ The `maxSize` parameter has been changed to `size` 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 than `maxSize`, and
+ macOS would always set the size to `maxSize`. Behavior is now the same across platforms.
+ 
+ Updated Behavior:
+ 
+ ```js
+ // a 128x128 image.
+ const imagePath = path.join('path', 'to', 'capybara.png')
+ 
+ // Scaling up a smaller image.
+ const upSize = { width: 256, height: 256 }
+ nativeImage.createThumbnailFromPath(imagePath, upSize).then(result => {
+   console.log(result.getSize()) // { width: 256, height: 256 }
+ })
+ 
+ // Scaling down a larger image.
+ const downSize = { width: 64, height: 64 }
+ nativeImage.createThumbnailFromPath(imagePath, downSize).then(result => {
+   console.log(result.getSize()) // { width: 64, height: 64 }
+ })
+ ```
+ 
+ Previous Behavior (on Windows):
+ 
+ ```js
+ // a 128x128 image
+ const imagePath = path.join('path', 'to', 'capybara.png')
+ const size = { width: 256, height: 256 }
+ nativeImage.createThumbnailFromPath(imagePath, size).then(result => {
+   console.log(result.getSize()) // { width: 128, height: 128 }
+ })
+ ```
+ 
+ ### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`
+ 
+ `BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the
+ `BrowserWindow.setWindowButtonPosition(position)` API should be used instead
+ which accepts `null` instead of `{ x: 0, y: 0 }` to reset the position to
+ system default.
+ 
+ ```js
+ // Removed in Electron 24
+ win.setTrafficLightPosition({ x: 10, y: 10 })
+ win.setTrafficLightPosition({ x: 0, y: 0 })
+ 
+ // Replace with
+ win.setWindowButtonPosition({ x: 10, y: 10 })
+ win.setWindowButtonPosition(null)
+ ```
+ 
+ ### Deprecated: `BrowserWindow.getTrafficLightPosition()`
++>>>>>>> refactor: createThumbnailFromPath takes size not maxSize

### API Changed: `nativeImage.createThumbnailFromPath(path, size)`

The `maxSize` parameter has been changed to `size` 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 than `maxSize`, and
macOS would always set the size to `maxSize`. Behavior is now the same across platforms.

Updated Behavior:

```js
// a 128x128 image.
const imagePath = path.join('path', 'to', 'capybara.png')

// Scaling up a smaller image.
const upSize = { width: 256, height: 256 }
nativeImage.createThumbnailFromPath(imagePath, upSize).then(result => {
console.log(result.getSize()) // { width: 256, height: 256 }
})

// Scaling down a larger image.
const downSize = { width: 64, height: 64 }
nativeImage.createThumbnailFromPath(imagePath, downSize).then(result => {
console.log(result.getSize()) // { width: 64, height: 64 }
})
```

Previous Behavior (on Windows):

```js
// a 128x128 image
const imagePath = path.join('path', 'to', 'capybara.png')
const size = { width: 256, height: 256 }
nativeImage.createThumbnailFromPath(imagePath, size).then(result => {
console.log(result.getSize()) // { width: 128, height: 128 }
})
```

### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`

`BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the
Expand Down
19 changes: 9 additions & 10 deletions shell/common/api/electron_api_native_image_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(

if (FAILED(hr)) {
promise.RejectWithErrorMessage(
"failed to create IShellItem from the given path");
"Failed to create IShellItem from the given path");
return handle;
}

Expand All @@ -53,36 +53,35 @@ v8::Local<v8::Promise> NativeImage::CreateThumbnailFromPath(
IID_PPV_ARGS(&pThumbnailCache));
if (FAILED(hr)) {
promise.RejectWithErrorMessage(
"failed to acquire local thumbnail cache reference");
"Failed to acquire local thumbnail cache reference");
return handle;
}

// Populate the IShellBitmap
Microsoft::WRL::ComPtr<ISharedBitmap> pThumbnail;
WTS_CACHEFLAGS flags;
WTS_THUMBNAILID thumbId;
hr = pThumbnailCache->GetThumbnail(pItem.Get(), size.width(),
WTS_FLAGS::WTS_NONE, &pThumbnail, &flags,
&thumbId);
hr = pThumbnailCache->GetThumbnail(
pItem.Get(), size.width(),
WTS_FLAGS::WTS_SCALETOREQUESTEDSIZE | WTS_FLAGS::WTS_SCALEUP, &pThumbnail,
NULL, NULL);

if (FAILED(hr)) {
promise.RejectWithErrorMessage(
"failed to get thumbnail from local thumbnail cache reference");
"Failed to get thumbnail from local thumbnail cache reference");
return handle;
}

// Init HBITMAP
HBITMAP hBitmap = NULL;
hr = pThumbnail->GetSharedBitmap(&hBitmap);
if (FAILED(hr)) {
promise.RejectWithErrorMessage("failed to extract bitmap from thumbnail");
promise.RejectWithErrorMessage("Failed to extract bitmap from thumbnail");
return handle;
}

// convert HBITMAP to gfx::Image
BITMAP bitmap;
if (!GetObject(hBitmap, sizeof(bitmap), &bitmap)) {
promise.RejectWithErrorMessage("could not convert HBITMAP to BITMAP");
promise.RejectWithErrorMessage("Could not convert HBITMAP to BITMAP");
return handle;
}

Expand Down
24 changes: 24 additions & 0 deletions spec/api-native-image-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,30 @@ describe('nativeImage module', () => {
const result = await nativeImage.createThumbnailFromPath(goodPath, goodSize);
expect(result.isEmpty()).to.equal(false);
});

it('returns the correct size if larger than the initial image', async () => {
// capybara.png is a 128x128 image.
const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
const size = { width: 256, height: 256 };
const result = await nativeImage.createThumbnailFromPath(imgPath, size);
expect(result.getSize()).to.deep.equal(size);
});

it('returns the correct size if is the same as the initial image', async () => {
// capybara.png is a 128x128 image.
const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
const size = { width: 128, height: 128 };
const result = await nativeImage.createThumbnailFromPath(imgPath, size);
expect(result.getSize()).to.deep.equal(size);
});

it('returns the correct size if smaller than the initial image', async () => {
// capybara.png is a 128x128 image.
const imgPath = path.join(fixturesPath, 'assets', 'capybara.png');
const maxSize = { width: 64, height: 64 };
const result = await nativeImage.createThumbnailFromPath(imgPath, maxSize);
expect(result.getSize()).to.deep.equal(maxSize);
});
});

describe('addRepresentation()', () => {
Expand Down
Binary file added spec/fixtures/assets/capybara.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.