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

Drag 'n drop #551

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Drag 'n drop #551

merged 1 commit into from
Dec 18, 2024

Conversation

jelly
Copy link
Member

@jelly jelly commented Jun 13, 2024

@tomasmatus interested?

One tricky thing here is we already have an upload-button component, so the eventlistener should be set up there instead so we don't have to pass files around.


Drag-and-Drop file upload

It is now possible to upload files by simply dragging them from your desktop or a file explorer and dropping them directly into Cockpit Files.

dragndrop-smaller

src/upload-button.tsx Fixed Show fixed Hide fixed
@jelly
Copy link
Member Author

jelly commented Jul 16, 2024

This works now, some pain points:

  • We should disable dropping when an upload is in progress
  • Chrome at least shows that one can drop everywhere which is not the case

Test on:

  • Firefox
  • Webkit

@tomasmatus
Copy link
Member

tomasmatus commented Oct 22, 2024

ok so I think that broke uploading when it should work (when using drag & drop)

@tomasmatus
Copy link
Member

tomasmatus commented Nov 19, 2024

oh right, tests won't work because I made some local changes to pkg/lib.

Also there's a bug I am currently trying to debug - drag&drop doesnt upload to pwd but to some old path? (maybe some old state/props, idk, debugging)

src/upload-button.tsx Fixed Show fixed Hide fixed
tomasmatus added a commit to tomasmatus/cockpit that referenced this pull request Nov 21, 2024
This enables to simulate drag events and allows to test drag&drop file
upload in c-files in cockpit-project/cockpit-files#551
tomasmatus added a commit to tomasmatus/cockpit that referenced this pull request Nov 21, 2024
This enables to simulate drag events and allows to test drag&drop file
upload in c-files in cockpit-project/cockpit-files#551
@tomasmatus
Copy link
Member

tomasmatus commented Nov 21, 2024

@garrett I think we can now start talking about how to show that files can be dropped for upload. Right now I have this:

image

Take it as a concept screenshot, I needed something that "works" and is not a border-color: red.

The upload icon appears when user is dragging some file over the files card. Maybe it should instead work on the whole page.
You said you have some ideas how we could design this so I'd appreciate if we could discuss this next week.

src/files-card-body.tsx Fixed Show fixed Hide fixed
@tomasmatus tomasmatus force-pushed the drag-n-drop branch 2 times, most recently from 222e2b8 to b946f6c Compare November 25, 2024 10:54
@garrett
Copy link
Member

garrett commented Nov 26, 2024

So this is a quick attempt to mock up something.

Notes:

  1. Not pixel perfect 😉 ... pixels will be off; sizes are probably off too; the font size is just some arbitrary one in Inkscape, not a PF size (and we should probably use a PF size)
  2. Large drop area, over the folder view, so it's a big, easy to achieve drop target
  3. 80% opaque background to provide a background contrast to the upload text, but transparent enough to show the files are still there
  4. Directory path at the top and summary at the bottom are not obstructed, making it possible to identify where you're dropping it and that there are other files in that directory
  5. Since the directory path and the status are not a drop target, they're a fine place to hover over to cancel the drop operation
  6. Canceling the drop operation (not dropping on the target) should not load the file; it should prevent the default of browser dragging and dropping from loading the file... ideally this cancelling would also work when dropping on the shell, but I know there are probably technical reasons why this would be difficult
  7. Singular vs. plural file/files depending on what's in the hover stack
  8. This mockup doesn't show the cursor; I don't know if that's handled by default
  9. The outline is dashed 3px PF link color
  10. The overlay should have the same size as the card, so it probably needs to be a sibling that comes directly after the card's content and either have the same grid position... or be positioned absolutely.

I'll probably need to help with the CSS.

drop-file-to-upload

Source: drop-file-to-upload.svg

WDYT?

@tomasmatus
Copy link
Member

I think that looks good, I like it! Thanks for the mockup

@tomasmatus tomasmatus added no-test and removed no-test labels Dec 2, 2024
@tomasmatus tomasmatus marked this pull request as ready for review December 3, 2024 12:35
@tomasmatus
Copy link
Member

tomasmatus commented Dec 3, 2024

  1. Singular vs. plural file/files depending on what's in the hover stack

I'm afraid this is not possible, the DataTransfer object is added only on dragdrop event, for dragenter, dragover or dragleave it's set to null.

test/check-application Outdated Show resolved Hide resolved
src/files-card-body.tsx Outdated Show resolved Hide resolved
@jelly
Copy link
Member Author

jelly commented Dec 4, 2024

I would also suggest showing that you can't drop a file when an upload is in progress, to show a different backdrop which explains an upload is in progress.

@tomasmatus
Copy link
Member

tomasmatus commented Dec 9, 2024

I would also suggest showing that you can't drop a file when an upload is in progress, to show a different backdrop which explains an upload is in progress.

Or we can not block the dropzone/upload button when there is another upload in progress

EDIT:
Before we actually go forward with never blocking the upload we'd need to look into it more closely. Discussed with @jelly and we'll probably postpone that for next year and for now keep the current blocking behavior

@tomasmatus tomasmatus force-pushed the drag-n-drop branch 2 times, most recently from aa8e935 to a06532e Compare December 16, 2024 08:51
@tomasmatus tomasmatus force-pushed the drag-n-drop branch 4 times, most recently from a6ddbf5 to 9c1e070 Compare December 17, 2024 15:55

export const UploadContext = createContext({
uploadedFiles: {},
setUploadedFiles: () => console.warn("UploadContext not initialized!"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test.

Copy link
Member

@tomasmatus tomasmatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! 🚀

@jelly jelly merged commit 75ba024 into cockpit-project:main Dec 18, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants