-
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
feat(file-uploader): add drag and drop file uploader #3175
feat(file-uploader): add drag and drop file uploader #3175
Conversation
Deploy preview for carbon-elements ready! Built with commit 09db8a5 |
Deploy preview for the-carbon-components ready! Built with commit 09db8a5 https://deploy-preview-3175--the-carbon-components.netlify.com |
Deploy preview for carbon-components-react ready! Built with commit 09db8a5 https://deploy-preview-3175--carbon-components-react.netlify.com |
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.
🎉 - Just one comment. Thanks @emyarod!
Does this supersede #2848? |
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.
Great! Only a few details I noticed. And thanks to @asudoh 's theme switcher, now I can review in all themes!
-
file name is the wrong color in error state. Should be
$text-01
or Gray100
-
Dark themes, success icon's inside checkmark should be a white fill. Also success icon's blue color should be coming from token
interactive-04
, which would have Blue50 for dark themes.
-
Dark theme, error state's close X might have the wrong color token? Should be
$icon-02
Specific for react:
-
the little green plus sign on user's cursor should only show when the cursor dragging a file is inside the drop zone. Cursor should remain default when dragging outside the drop zone.
-
the blue success icon is on for too long before turning into an X. Vanilla's version is okay, probably about 1 second is good.
Side note, I'd love to add some motion design to this component, but I'm not sure how it impacts @asudoh and @emyarod your works. Maybe this is what Josh is asking too. Does one need to be updated before another? Are the two PR dependent on each other? Specifically, I'm thinking it would be great to add motion to the file list item (appearance & error state), and, transition of loading spinner to success icon to X. I will post a prototype tomorrow. |
Thank you for the heads-up @shixiedesign! This PR contains my earlier work (#2848). |
29f5d1f
to
3516ab5
Compare
@shixiedesign all points have been addressed except for the React cursor changing/not changing. I will need to look more into this |
2d689b2
to
810b079
Compare
React component cursor changes should now be resolved |
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.
Great to see this PR in ready-for-review state @emyarod!
packages/react/src/components/FileUploader/FileUploaderDropContainer.js
Outdated
Show resolved
Hide resolved
packages/react/src/components/FileUploader/FileUploaderDropContainer.js
Outdated
Show resolved
Hide resolved
d2db92f
to
2e80764
Compare
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.
Looks good! 💯Thanks for making the fix!
b4dca27
to
09db8a5
Compare
Should we close this one in favor of #3872? |
Closes #2088
Closes #2288
Closes #2916
Closes #2917
Closes #3252
refs #2848 #3016
This PR adds drag and drop support to the vanilla and React file uploader component, along with various bugfixes
Changelog
New
<FileUploaderDropContainer>
component to represent drop area for file uploads<FileUploaderItem>
component to represented uploaded itemsChanged
<Filename>
to stateless functional componentRemoved
<InlineLoading />
(to make use of fix(Loading): use description prop as SVG title #2955)Testing / Reviewing
Ensure the new file uploader functions as expected and ensure the existing file loader does not have any regressions