-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Unused code in FileUploader #2631
Comments
Thanks @emyarod for reporting! I feel I’m kind of one the fence - The React one shows a nice UI wrt what is being uploaded, but loses a context of additional files can be uploaded. CC @carbon-design-system/design to see which one is more desirable. |
hi @asudoh I am leaning towards the current behavior found in the vanilla component (not changing the text within the button), but I am fine with either option. I just want more clarification on the direction we want to proceed with as I fix the dead code issue. One question though: is the Right now in the Carbon React storybook, the |
The button text should not be changing, but stay persistent, no matter how many files the user uploads. If the user wants to add more files after the initial upload, there are now no cues for how to go it. That shouldn't have been built into the react version of the component. |
@tay-aitken thanks for the response. The React version currently doesn't change its button text, but it does have some logic that (if properly connected) would cause that behavior. Since it shouldn't have been added to the component though, I will go ahead and remove the dead code |
thank you! Appreciate you doing that! |
is there a sandbox example with mock interactions for uploading files? The vanilla documentation for Reading through the current implementation though, it seems some of the features shown in the GIFs are not ideally implemented. It looks like file upload status is applied to all files in the component as opposed to being bound to each file individually (i.e. every file in the component will share the same status regardless of how far along it is in the upload process, or what the individual outcomes are of each upload). |
Hi @emyarod not sure if your first question is referring to the vanilla code or the React code, but here’s the way we test status updates in our vanilla implementation: The To the 2nd question, I think you are right that |
sorry @asudoh I mistook the vanilla API Just to get a better idea of how to use the current |
Thanks @emyarod for articulating your question further! I think the only thing I know is http://react.carbondesignsystem.com/?selectedKind=FileUploader&selectedStory=FileUploader&full=0&addons=1&stories=1&panelRight=0&addonPanel=storybook%2Factions%2Factions-panel - It'd be nice if you have some ideas on involving/adding the story. Thanks! |
@asudoh so I guess it is currently left up to the user to provide their own logic for making HTTP requests. This is fine as it is, if we want to keep the current behavior of applying 1 upload status to the entire upload operation. But if we want to have individual upload status for each file in the operation, would it be worth implementing that logic within |
Thanks alot @emyarod for articulating that you were looking for per-file upload status in our React variant and my apologies for not catching that! As vanilla supports that feature, I believe React should do that, too. Great to have that if it can be done in a non-breaking way. Thanks! |
@asudoh as we discussed on Slack, I will begin with V2 of the |
Thank you so much for doing this @emyarod! Just a note - |
We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. Thanks for your contributions. |
Hi there! 👋 If you're wondering why this issue was moved, we're currently updating our repo structure so that every package is found in the same project. This should not have any impact for you, but we wanted to give you a heads up in case you were wondering what is going on. If you have any questions, feel free to reach out to us on Slack or contact us at: [email protected]. Thanks! |
We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. You can keep the conversation going with just a short comment. Thanks for your contributions. |
Detailed description
while working on #1022 I noticed some divergence in the
FileUploader
from the vanilla components. Looking for some clarification on how to proceedWhen uploading files, the React
FileUploader
has some logic that will modify the label text. If the user is uploading multiple files, it will change the label text fromAdd files
to the number of files currently being uploaded, and if the user is uploading a single file, the label text will be changed to the name of the file being uploaded. A couple things about the relevant code though:should I remove it completely or is this a desirable feature extension from the vanilla implementation?
general issue
FileUploader
Right now I am working around this issue while refactoring the component, but it might not even be worth keeping since it is different from the vanilla implementation. Would like some input on this matter though.
6.6.3
Additional information
Current Vanilla and React
FileUploader
behavior:The following behavior is present within React
FileUploader
but currently unreachable. The idea seems to be that if the user is uploading multiple files, it will change the label text fromAdd files
to the number of files currently being uploaded, and if the user is uploading a single file, the label text will be changed to the name of the file being uploaded. Don't know if I agree with this behavior but if it needs to be modified I can add in the revised behavior. Otherwise I can remove the unreachable code:The text was updated successfully, but these errors were encountered: