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
Add File Dropzone component #813
base: next
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thank you for submitting this! I'm still trying to carve out time to give it a proper review. Please bear with me! |
Thanks for submitting this! For consistency, clarity, and familiarity, can we rename Drop Handler
I may have additional feedback on this one depending on your answers to the above questions :) File Dropzone
File Item
Other
AppreciationOK, that's a lot! 😅 Great work thus far. This is a complex component, and I'm thrilled to see such a thorough PR. A few final points, since these things always come across as criticism/negative and that's definitely not the intent!
|
Thank you @claviska for your detailed review!
We can do that. Altough you mention that the component should not actually have any upload functionality, so alternatively they could be named DropHandlerAfter some reconsideration we would suggest to remove the DropHandler component entirely and instead use native events in the FileDropzone. The current functionality does not seem to warrant a separate component. FileDropzone
It sounds like a good idea to reduce complexity in this component. We can remove the upload functionality and create examples in the documentation.
That would indeed be nice. In this case we would need to provide a way for the component to properly work in a form. Looking at the other form components and the documentation they seem to only instantiate a
Good point. We should then probably move the click listener further up so it triggers even when there is a different element in the slot.
We will make the standard button variant
Can you clearify what you mean with this?
We will rename
Good point. Further customization could also be achieved with native elements if we remove the DropHandler.
We will remove
We will add the icons to the system icon library.
We will leave the icon size of the FileItem the same when size is set.
We will inline the FileItem
We would like to add this in another PR so we can reduce the complexity. Would it be ok for you to add this as a Feature Request and get back to it later?
We will rename
We will adjust the spacing and font size. Other
Thank you very much. Your help would be appreciated. We are looking forward to your response. |
Sounds good!
This is my workaround for not having ElementInternals in all browsers yet. When the FormSubmit controller is linked to a component, a number of things happen when said component is connected to the DOM. https://github.com/shoelace-style/shoelace/blob/next/src/internal/form.ts#L55-L85 The controller taps into the When the controller is instantiated, you can pass options to customize how https://github.com/shoelace-style/shoelace/blob/next/src/internal/form.ts#L33-L53 Aside from instantiating and optionally mapping those properties, the FormSubmit controller handles the rest.
When the warning state is shown, everything turns amber. But if we allow the user to slot in buttons and such, we won't want to manipulate their buttons, meaning you could end up with something like this: So we should probably revisit the error styles to account for this.
Absolutely! And thanks again for the PR! |
@claviska Do you have an idea how to to handle upload progress? |
That would be provided by the XHR so the user will need to wire it up to their request. If they're using axios, for example: axios.request({
method: 'post',
url: '/upload',
data,
onUploadProgress: (p) => {
const percentage = p.loaded / p.total;
//
// pass percentage back to the uploader here
//
}
}).then(...); I'll admit it's less convenient than just providing a URL, but it allows the user to handle uploading with whatever utility they want and the component doesn't have to support all fetch/XHR options, including more complex features such as auth, resuming failed uploads, server-side validation conventions, handling various server-side error responses, chunking uploads, large file uploads, etc. To make things easier, I would recommend writing an adapter for the component that does that wiring for your API or library of choice. It's not something I'd prefer to maintain myself, though. |
@claviska We updated the discussed changes. Looking forward to any feedback! |
13900d6
to
cf7623a
Compare
@claviska Could you approve the deployment workflow for the new commits made by me, please? Also, FYI, as @mkitzmann is on vacation for next couple of weeks, I would be part of the discussions till he is back! :) Thank you in advance, and we are looking forward to your feedback on recent changes!! |
Approved, and apologies for the delay. I need to carve out some time to focus on this, which I promise to do soon! |
cf7623a
to
67bf3e3
Compare
@claviska Is there anything we can do to help with review process? |
This is 100% on me, and I sincerely apologize. I haven't had as much spare time as I'd like to tackle this, but I will be circling back to this PR soon! Thanks for your patience! |
@claviska were you able to look at our updates? (we are approaching some team changes over here and will probably have less time in the future for the design system ;( |
I'm really sorry. I had a big push at work a couple weeks ago, followed by a vacation during which I caught a nasty cold. 😭 I'm running behind on some things — this being one of them. I really appreciate the effort that's gone into this and want to give it a proper review as soon as I'm able. Thanks again for your patience! |
Please accept an explicit 👍 as a token of interest in this functionality |
Here's where I'm at with this PR — and I don't want to minimize anyone's contribution in any way — but I think since the scope was scaled down from the original submission, this can probably be implemented in a simpler way without the need for an additional file item component. I've been wanting to experiment with this to make sure (and hopefully I'll be able to soon), but my thought is that file items can be made part of the template with parts that the user can customize with CSS. The template will essentially mirror I can't foresee a need for users to declaratively populate a file input since they wouldn't have permission to access those files anyway without the user explicitly selecting them first. |
Thank you @claviska for your feedback. The other problem now is, that Im no longer part of SDA-SE and therefore dont have write access to that repo. I guess I could fork the repo and open another PR. I will contact my former colleagues about this just to be sure. |
Hi @claviska Thank you for your feedback. Sadly, as @mkitzmann is no longer with SDA-SE, he cannot make changes to our repo anymore! I will update the PR in couple of days with the mentioned changes. @mkitzmann If you wish, please feel free to open up a PR to against the SDA-SE FileDropZone branch ;-) And also please do the review after my changes if possible. :) |
Thanks, all. I have a big announcement to make soon, so please hold off on any work here for a little while. 😄 |
Here's a proof of concept for a simpler implementation that uses just one component. It's already wired up and submitting correctly. As you can tell, I haven't spent much time on the design and it's missing drag & drop, error handling, parts, etc., but this should bring some more clarity to my vision and why I've held off on this PR. If you view the preview, make sure to open the dev console when submitting to see the result. |
Hey @claviska, |
Let's hold off on this for now so I can plan things out with the team. We may decide to take this one in house out of necessity. Thanks all! Your efforts are much appreciated! |
@claviska any update from your side? |
Thanks for your patience, and apologies for the delay. Shoelace now has a full-time team of three people, including a fantastic designer, and we're going to start rolling out some of these core components very soon! |
Any news? It would be really great to merge this long awaited feature! 👀 As for my 2 cents feedback, I'd also suggest to make the whole dropzone a button instead of having an extra button "browse files" inside. ;) Nonetheless, great job guys! 👍 Edit: sorry, I read the whole thread and I see the big picture now. Basically, the PR was made ages ago and forever delayed because of its complexity. I can uderstand but it's a pity though. This hard work is at risk of being lost. Regarding your simplified component suggestion, it looks oversimplified to be honest. The important functionality here is the dropzone after all. |
We have a plan for this one to be finished and released early next year, along with a lot of great improvements to theming. Lots of great stuff is happening. Thanks for your patience! |
@claviska any news? This component is very important. |
Any updates on the timeline for this component? |
It seems that future additions to shoelace will only be available via payed subscription, and drop zone doesn't seem to be on the immediate to do list (source - see stretch goals) It seems that if someone in the community wants to create an open source component, the way to do that would be as part of a separate project (as it seems @klaarseSICKAG is doing). It would be convenient if there would be an official shoelace (or web awesome) community edition which would be quicker to accept community PRs, but
|
On the contrary, it’s not listed as a stretch goal because we’re planning on the file input to be part of free along with a number of other components. We plan on adding more to Web Awesome free just like Font Awesome free continues to add more icons. We may add a more complex uploader under pro, but we haven’t discussed specifics of that yet. (You may have noticed that most of the pro components are the more complex ones.)
Remember that you can use any web component you want with Shoelace/Web Awesome. The reason we’re offering a pro version is so we can keep building this stuff for you. Bear in mind that I had to take a bit of a break to assemble a team and get things in order. I appreciate your continued support and patience as we transition. |
Gotcha. |
This PR adds a File Dropzone as discussed in #118.
It consists of 3 seperate components:
The DropHandler is a utility component that simply enables drag-and-drop functionality for whatever is added inside its default slot.
The FileItem can be used to display a File including its filename and optionally its size. When the
size
attribute is defined it uses the FormatBytes component internally to display the bytes in a human-readable way.The FileDropzone component provides an area for files to be dropped as well as a button to open the file-select dialog. When a
url
attribute is specified the selected file will be uploaded to the given url. The FileDropzone works for multiple Files by setting themax-files
attribute. Each selected file is displayed below the dropzone using the FileItem component. Warnings and transfer errors will be shown separately for each file.We have decided to not build a FileItemList component, since it would not have enough functionality to create a separate component.
We have created documentation and tests for each component.
We are looking forward to any feedback!