[DevTools][AI Assistant] Allow Users To Paste Images Directly From Clipboard [devtools/devtools-frontend : main]
Samiya Caur (Gerrit)
Samiya Caur added 2 comments![]( "Open in Gerrit")
Thank you! Can you also look into adding a test for this?
#handlePaste = (event: ClipboardEvent): void => {nit: since we only handle pasting of image, can we rename this to something more specific?
same for `onPaste`
Related details
- Emir D
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emir D (Gerrit)
Emir D added 3 comments![]( "Open in Gerrit")
Thank you! Can you also look into adding a test for this?
Hi, I updated the related codes and added the tests to ChatInput.test.ts file.
nit: since we only handle pasting of image, can we rename this to something more specific?
same for `onPaste`
Hi, I changed the related method names and maked more informational and specific. See the changes;
- ChatInput.ts:
- onPaste -> onImagePaste
- handlePaste -> handleImagePaste
- ChatInput.test.ts:
- onPaste -> onImagePaste
Related details
- Samiya Caur
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Samiya Caur (Gerrit)
Samiya Caur voted![]( "Open in Gerrit")
- Emir D
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ergün Erdoğmuş (Gerrit)
Ergün Erdoğmuş added 5 comments![]( "Open in Gerrit")
const dataTransferItem = {We can directly construct `DataTransfer` object here and give it to the `ClipboardEvent` as `clipboardData` instead of doing type assertions through `unknown`.
Something like this should work out:
```
const file = new File(['test'], 'pasted_image.png', {type: 'image/png'});
const dataTransfer = new DataTransfer();
dataTransfer.items.add(file);
const clipboardEvent = new ClipboardEvent('paste', {
clipboardData: dataTransfer,
});
``` data: 'dGVzdA==',(super nit): Let's use `data: btoa('test')` to provide a link between the used `File` and the test assertion.
fileReaderStub.restore();We have an `afterEach` that restores all the stubbed methods, we don't need to call this specifically; feel free to remove this call :)
for (const item of items) {(nit): We can avoid the imperative loop here. When we iterate with `continue`, we force the reader to parse the control flow to understand the goal.
If we use `Array.from(items).find(...)`, the code becomes "declarative". It describes what we want (an image item) rather than how to loop through the list.
===
I have also realized that we can use the `files` property from `clipboardData` too, so we won't need calling `getAsFile()` if we find the files directly from there. Let's use that to find the image file.
Related details
- Emir D
- Samiya Caur
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emir D (Gerrit)
Emir D added 5 comments![]( "Open in Gerrit")
The patchset 3 has been published due to last comments.
We can directly construct `DataTransfer` object here and give it to the `ClipboardEvent` as `clipboardData` instead of doing type assertions through `unknown`.
Something like this should work out:
```
const file = new File(['test'], 'pasted_image.png', {type: 'image/png'});
const dataTransfer = new DataTransfer();
dataTransfer.items.add(file);const clipboardEvent = new ClipboardEvent('paste', {
clipboardData: dataTransfer,
});
```
Related file has been edited. Now we are constructing DataTransfer here. We are not now using 'unknown' case.
(super nit): Let's use `data: btoa('test')` to provide a link between the used `File` and the test assertion.
I edited the test data with btoa() method to provide a link between the used File and the test assertion.
We have an `afterEach` that restores all the stubbed methods, we don't need to call this specifically; feel free to remove this call :)
I removed this call. I'm trying to get used to Google culture :)
(nit): We can avoid the imperative loop here. When we iterate with `continue`, we force the reader to parse the control flow to understand the goal.
If we use `Array.from(items).find(...)`, the code becomes "declarative". It describes what we want (an image item) rather than how to loop through the list.
===
I have also realized that we can use the `files` property from `clipboardData` too, so we won't need calling `getAsFile()` if we find the files directly from there. Let's use that to find the image file.
I edited the ChatView.ts file and now we are using Array.from() method to check files in clipboard. Also now, we are getting the image from files property which in clipboardData.
Related details
- Ergün Erdoğmuş
- Samiya Caur
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ergün Erdoğmuş (Gerrit)
Ergün Erdoğmuş voted Code-Review+1![]( "Open in Gerrit")
- Emir D
- Samiya Caur
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |