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

feat(Upload): support files dropped on the document body #1719

Merged
merged 5 commits into from
Nov 17, 2022

Conversation

tujoworker
Copy link
Member

@tujoworker tujoworker commented Nov 14, 2022

I think its better to make this not optional.

But then the question is, should only the first support it?

If yes, then we probably want a way to support via a property.
Or should it be possible to disable?

But probably, its fine as of now. Having several upload components on the page is probably something that not should happen.

@tujoworker tujoworker requested a review from langz November 14, 2022 09:53
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 14, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit bb55a18:

Sandbox Source
eufemia-starter Configuration

@gatsby-cloud
Copy link

gatsby-cloud bot commented Nov 14, 2022

✅ DNB Eufemia Portal deploy preview ready

Copy link
Contributor

@langz langz left a comment

Choose a reason for hiding this comment

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

Some of the existing functionality don't seem to work flawlessly now:

  • No yellow animation when uploading the same file multiple times by dropping files in other places than the "upload zone".
  • Seems like when dropping a file in other places than the "upload zone" it's replacing all the existing files with the current dropped file, and not adding new/more files.

No yellow animation:

Screen.Recording.2022-11-14.at.12.39.18.mov

Replacing existing file instead of adding files:

Screen.Recording.2022-11-14.at.12.40.27.mov

macOS: 12.6 (21G115)
Chrome: Version 107.0.5304.110 (Official Build) (x86_64)

@langz
Copy link
Contributor

langz commented Nov 14, 2022

Good questions.
I'm trying to think of a scenario where apps don't want to get this functionality by default, but can't think of any 🤔

Having several upload components on the page is probably something that not should happen.
I could think of large forms, where it probably makes sense to have multiple upload components on a page, to separate which category/section the files belong to. Or maybe I've just created a scenario for it in my head now 😂

@tujoworker tujoworker force-pushed the feat/upload-page-listener branch from 79a4d29 to bb55a18 Compare November 14, 2022 14:30
@tujoworker
Copy link
Member Author

@langz very nice finding! That's a wired React Hook issue, so for now I put the files into a ref and update the ref on each re-render. I have had this before in other places where we use Hooks. Probably a React bug. But the reason was that useUpload did return an empty list, even its not empty in the hook.

@tujoworker
Copy link
Member Author

@langz also, now only the first one will accept files from the "body" – what do you think? Is that better or worse? We could maybe add an property instead dropFilesSelector and default to body? This way, its possible to define a HTML selector – or disable it, if null/undefined was given. Just an idea.

@langz
Copy link
Contributor

langz commented Nov 15, 2022

@langz also, now only the first one will accept files from the "body" – what do you think? Is that better or worse? We could maybe add an property instead dropFilesSelector and default to body? This way, its possible to define a HTML selector – or disable it, if null/undefined was given. Just an idea.

Thanks for improving this. In the possible scenario of where there's multiple upload components in a single page, I do think it will cause less confusion/damage for the user to only add the files dragged to "body" to one of the upload components, than to all of the upload components.

In general, if there's multiple upload components in a single page, I do think it would be favourable that the developer could at least select which upload component that will get the files that's been dragged to "body".
Let's say if it's somehow super important that the 2nd upload component on my page should get the files dragged to "body", then I would have to make sure that the 2nd upload component would mount 1st to make this happen. That wouldn't result in meaningful code 😂
I would say that this is probably an edge case, and that we can handle this later if we wish.

@langz
Copy link
Contributor

langz commented Nov 15, 2022

Works perfectly fine on iPhone as well👌
https://user-images.githubusercontent.com/1359205/201878970-22ff3870-1a73-4621-841d-c66fa92b1080.MOV

@tujoworker
Copy link
Member Author

Works perfectly fine on iPhone as well👌

ohh, I didn't know that this was possible 😁

@tujoworker tujoworker merged commit f206243 into main Nov 17, 2022
@tujoworker tujoworker deleted the feat/upload-page-listener branch November 17, 2022 08:42
tujoworker pushed a commit that referenced this pull request Nov 17, 2022
# [9.38.0-beta.1](v9.37.0...v9.38.0-beta.1) (2022-11-17)

### Bug Fixes

* add support to IS_SAFARI_DESKTOP for Safari v16 on macOS ([#1718](#1718)) ([54e2cba](54e2cba))
* **Anchor:** export types as AnchorAllProps and original instance ([#1715](#1715)) ([92ec784](92ec784))
* **Icons:** prevent icons from having same IDs (duplicate-id violation) ([#1714](#1714)) ([5e4079d](5e4079d))
* **Provider:** rewrite to functional component ([#1731](#1731)) ([b504d06](b504d06))
* **Table:** align odd/even modifiers with CSS nth ([#1724](#1724)) ([8bdad07](8bdad07))

### Features

* **Table:** add "fixed" prop for fixed table layouts ([#1708](#1708)) ([241ee0f](241ee0f))
* **Table:** add Table.SortButton ([#1709](#1709)) ([288a8db](288a8db))
* **Table:** add Th.HelpButton to be used in Table Headers ([#1711](#1711)) ([c142323](c142323))
* **Th:** add table header sortable props ([#1706](#1706)) ([c40393a](c40393a))
* **Tr:** automate odd/even and make it overridable ([#1705](#1705)) ([d73d3cb](d73d3cb))
* **Upload:** support files dropped on the document body ([#1719](#1719)) ([f206243](f206243))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 9.38.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

tujoworker pushed a commit that referenced this pull request Nov 22, 2022
# [9.38.0](v9.37.0...v9.38.0) (2022-11-22)

### Bug Fixes

* add support to IS_SAFARI_DESKTOP for Safari v16 on macOS ([#1718](#1718)) ([54e2cba](54e2cba))
* **Anchor:** export types as AnchorAllProps and original instance ([#1715](#1715)) ([92ec784](92ec784))
* **Icons:** prevent icons from having same IDs (duplicate-id violation) ([#1714](#1714)) ([5e4079d](5e4079d))
* **Provider:** rewrite to functional component ([#1731](#1731)) ([b504d06](b504d06))
* **Table:** align odd/even modifiers with CSS nth ([#1724](#1724)) ([8bdad07](8bdad07))

### Features

* **Table:** add "fixed" prop for fixed table layouts ([#1708](#1708)) ([241ee0f](241ee0f))
* **Table:** add table "border" and "outline" property ([#1739](#1739)) ([ad63ffb](ad63ffb))
* **Table:** add Table.ScrolView to support horizontal scroll ([#1735](#1735)) ([85a4d86](85a4d86))
* **Table:** add Table.SortButton ([#1709](#1709)) ([288a8db](288a8db))
* **Table:** add TableContainer to stack tables with an outline ([#1740](#1740)) ([376ac06](376ac06))
* **Table:** add Th.HelpButton to be used in Table Headers ([#1711](#1711)) ([c142323](c142323))
* **Table:** support rowSpan ([#1733](#1733)) ([463692d](463692d))
* **Th:** add table header sortable props ([#1706](#1706)) ([c40393a](c40393a))
* **Tr:** automate odd/even and make it overridable ([#1705](#1705)) ([d73d3cb](d73d3cb))
* **Typography:** support styles for superscript and subscript elements ([#1721](#1721)) ([c2b043d](c2b043d))
* **Upload:** support files dropped on the document body ([#1719](#1719)) ([f206243](f206243))
@tujoworker
Copy link
Member Author

🎉 This PR is included in version 9.38.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

joakbjerk pushed a commit that referenced this pull request Mar 27, 2023
* feat(Upload): support files dropped on the document body

* Add UploadDragEvent type

* Support only one (the first one)

* Enhance handling of existing files and their ID

* Fix existing file handling when file get dropped on body
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants