-
Notifications
You must be signed in to change notification settings - Fork 32
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
fix(Field.Upload): handling of multiple async file uploads #4360
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/dnb-eufemia/src/extensions/forms/Field/Upload/__tests__/Upload.test.tsx
Outdated
Show resolved
Hide resolved
…__/Upload.test.tsx
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. |
packages/dnb-eufemia/src/extensions/forms/Field/Upload/Upload.tsx
Outdated
Show resolved
Hide resolved
isLoading: false, | ||
}) | ||
|
||
const indexOfFirstNewFile = filesRef.current.findIndex( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason we do it that way? What if there is one file added already. And later the user adds two. Will the second one ever be newFiles[0]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably do it in a different way, somehow this is the only one that made sense to me at the given time.
What I do is that I find the first "new file", and use that to find the index of where the "new files" is located in the files (the new files is always located/positioned after each other). I then remove/replace these new files with the async received files.
So what I want to do is to update the latest files by replacing the "new files" (the files newly added by the user) with the async received files, and keeping the same order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. Can you try if this works as well? I think it should do the same as the slicing:
const updatedFiles = filesRef.current.map((file) => {
const matchingNewFile = uploadedFiles.find(({ id }) => id === file.id)
return matchingNewFile || file
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great suggestion.
The problem is that the id
of the uploaded/async file can be something other than the local id
, as it can be server generated, as in the example setting server_generated_id
.
But it probably exists a more delicate way of solving this, than what I've done so far 🧠
Maybe using a different property? But then again, the response from the server could potentially change whatever property it wants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it does not the same 😀 – but I think now we are on something:
// Update files, replacing newFiles with uploadedFiles, preserving order
const updatedFiles = [
...filesRef.current.map(
(file) =>
uploadedFiles.find((uploaded) => uploaded.id === file.id) ||
file
),
...uploadedFiles.filter(
(uploaded) =>
!filesRef.current.some((file) => file.id === uploaded.id)
),
]
But it's not less code, so I'm not sure it's better 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll merge now, then try this improvement later.
I see that this again also depends on the id, which I again think can be different in what's coming from the "server".
packages/dnb-eufemia/src/extensions/forms/Field/Upload/__tests__/Upload.test.tsx
Show resolved
Hide resolved
Co-authored-by: Tobias Høegh <[email protected]>
packages/dnb-eufemia/src/extensions/forms/Field/Upload/__tests__/Upload.test.tsx
Outdated
Show resolved
Hide resolved
…__/Upload.test.tsx
## [10.60.0](v10.59.0...v10.60.0) (2024-12-10) ### 📝 Documentation * **Upload:** adds `exists` to file object ([#4346](#4346)) ([52f8b2f](52f8b2f)) * **Upload:** adds exists to docs ([b6baa64](b6baa64)) ### ✨ Features * **Field.Upload:** add `onFileClick` event ([#4369](#4369)) ([c892eec](c892eec)) * **Forms:** add `Form.InfoOverlay` to display error, success (receipt), or custom messages to users ([#4357](#4357)) ([9dd8402](9dd8402)) * **Forms:** add `onAnimationEnd` property to Form.Visibility ([#4356](#4356)) ([87728b4](87728b4)), closes [#4350](#4350) * **Forms:** add `onVisible` property to Form.Visibility ([#4350](#4350)) ([41306d8](41306d8)) * host fonts in the public directory ([#4359](#4359)) ([e6e08b2](e6e08b2)) * **Upload:** add support for async `onFileClick` event ([#4370](#4370)) ([82588c1](82588c1)) * **Upload:** adds `onFileClick` event ([#4365](#4365)) ([c5abd0e](c5abd0e)) * **Upload:** adds support for async `onFileDelete` ([#4351](#4351)) ([f41e42d](f41e42d)) * **Value.Upload:** add `onFileClick` event ([#4367](#4367)) ([56e9caf](56e9caf)) ### 🐛 Bug Fixes * **Dropdown:** enhance height calcilation and add support for strict `direction="bottom"` usage, including when used in a Dialog component ([#4368](#4368)) ([32b7b5b](32b7b5b)) * **Field.Upload:** handling of multiple async file uploads ([#4360](#4360)) ([5cb1518](5cb1518)) * **Forms:** avoid unnecessary rerenders in Form.Handler ([#4363](#4363)) ([7de5e49](7de5e49)), closes [#4357](#4357)
🎉 This PR is included in version 10.60.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
From:
Screen.Recording.2024-12-05.at.13.37.33.mov
To:
Screen.Recording.2024-12-05.at.13.16.22.mov