Repository navigation
Conversation
The explorer accepts a drop of files from the desktop, but nothing can be dragged out of it. Anyone who wants an object's URI somewhere else — a notebook cell, a chat box, a form field, an application embedding Onyxia — has to open the row menu, copy the URI, and paste it. Rows are now draggable. A drag publishes the S3 URIs twice: as `text/plain`, newline separated, which is what makes a drop onto an ordinary text field useful without the target knowing anything about Onyxia; and as `application/x-onyxia-s3-objects`, JSON, for a consumer that needs to tell "these are S3 objects" from "the user dropped some text". Nothing is read and nothing is modified — only the URIs travel. Three details that are easy to get wrong, so they are pure functions with tests rather than logic inside the component: - Which rows a drag carries is not "the selection". Dragging an unselected row drags that row alone; returning the selection there would carry files the user cannot see they had selected, since this list is virtualized. - Rows mid-upload or mid-delete are not draggable, and are dropped from a multi-row drag rather than refusing the whole gesture. - Detection during dragover reads `types` and never `getData()`. The DataTransfer is in protected mode while a drag is in flight, so every value reads back empty — a check written against `getData()` passes its unit test and refuses every real drag. A drag beginning on the row checkbox or a row action button is that control's gesture and does not become a row drag. Signed-off-by: Islam Eldemery <islam.eldemery@gmail.com>
…efix The explorer can list, upload, download, delete and share, but it cannot copy. There is no copy, no move and no rename anywhere in the app, and `S3Client` had no method for it — so getting a file from one prefix to another means downloading it and uploading it again, through the browser, at the size of the object. Objects and prefixes dragged out of an explorer can now be dropped onto a prefix row, or onto the surface to land in the listed prefix. It is CopyObject, server side: no bytes reach the browser, so a 40 GB object costs the client one request. Most of what makes this feel like a file manager is key arithmetic, so it is in a pure module with tests rather than inside the thunk: - Dragging a folder keeps its name and its internal shape. The part of the key below the dragged item's parent is what gets rebuilt under the destination — taking the basename instead would flatten every object side by side and let same-named files in different folders overwrite each other. - A prefix dropped into itself or into its own descendant is refused: the crawl would keep finding the objects it had just written. The check is on path segments, so `reports/` may still be copied into the sibling `reports-archive/`, which a string-prefix check would forbid. - Dropping something back where it already is is refused rather than performed; on a versioned bucket it would be a new version for nothing. `CopySource` is encoded per segment, which the AWS SDK does not do for this parameter — it is a header value, not a path parameter. An unencoded key containing a space, a `+` or a `#` either fails the request or copies a different object than the one asked for. Deliberately not here: - MOVE. It would be copy-then-delete with no transaction and no undo, so a partial failure loses data. It deserves its own design rather than riding in on a drag gesture. - Objects above 5 GiB, where S3 requires a multipart copy. The single-part limit is stated on the port so a caller keeps it rather than discovers it. An object row is not a destination: dragging across one on the way to the background falls through to the listed prefix rather than refusing the drop. The destination highlight is an outline rather than a fill, because the row beneath may already be selected or striped. A drag of files from outside the browser remains an upload and stays visually distinct — different gesture, different outcome. Depends on the row-drag change that publishes what is being dragged. Signed-off-by: Islam Eldemery <islam.eldemery@gmail.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| }) => { | ||
| const { sourceS3Uri, destinationS3Uri } = params; | ||
|
|
||
| const cmdId = Date.now() + Math.random(); |
Contributor
|
@idemery thank you very much for the PR. We'll discuss it with the team soon. On a scale of one to ten, how automated are the PR that you sumitted? I'm not saying that fully automated PR have no value, tokens are expensive, but I don't review human an LLM code in the same way. Best, |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Objects and prefixes dragged out of an explorer can be dropped onto a prefix row, or onto the surface to land in the listed prefix. It is
CopyObject, server side.Why
The explorer can list, upload, download, delete and share, but it cannot copy. There is no copy, no move and no rename anywhere in the app, and
S3Clienthad no method for it — so getting a file from one prefix to another means downloading it and uploading it again, through the browser, at the size of the object.Here no bytes reach the browser: a 40 GB object costs the client one request.
The key arithmetic
Most of what makes this feel like a file manager is path handling, so it lives in a pure module with tests rather than inside the thunk:
reports/may still be copied into the siblingreports-archive/, which a string-prefix check would forbid.CopySourceis encoded per segment, which the SDK does not do for this parameter — it is a header value, not a path parameter. An unencoded key containing a space, a+or a#either fails the request or copies a different object than the one asked for.Deliberately not here
Happy to follow up with either if you'd like them.
Interaction details
onCopyObjectsleft undefined makes the explorer a drag source only.Notes
copyObjecton theS3Clientport, one adapter implementation.S3ExplorerMainView.spec.mdupdated.tsc,eslintandvite buildclean.