-
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): support drag and drop #2848
feat(file-uploader): support drag and drop #2848
Conversation
Deploy preview for the-carbon-components ready! Built with commit 2564d2d https://deploy-preview-2848--the-carbon-components.netlify.com |
Deploy preview for carbon-components-react ready! Built with commit 2564d2d https://deploy-preview-2848--carbon-components-react.netlify.com |
a2a1dd8
to
a8411e7
Compare
Hi @asudoh! Thanks for starting on this issue.
|
ecb6b08
to
ff9972b
Compare
@shixiedesign Thank you for the design review! - Updated the demo link with most of suggested design change, except the width of the drop container given the original one is wider than 256px. Please let me know if shortening to 256px is more desirable. I'll get in sync with @emyarod on React implementation. Until then, let me keep this PR draft. |
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.
Hi @asudoh I spec'ed out the file loader more completely after finding the sketchfile, so a few more small visual fixes:
-
Default width of the drop zone, let's not make it smaller, but make it 320px so it's a nice number divisible by 8.
-
this loader should be 16x16 in overall size, with a ring thickness of 3px
Current
Should be
-
cursor should turn into "pointing hand" when hovering over the link for upload:
-
bug: when dragging+hovering, the blue outline shouldn't cause the text inside to move.
-
do we have an error state for the file list? (see below spec img) We should use
$support-01
for main error message, detailed error message is$text-01
, type size 12px,ui-03
for divider line.
Below is the complete spec for your reference:
ff9972b
to
d704ed5
Compare
Deploy preview for carbon-elements ready! Built with commit 2564d2d |
d704ed5
to
036a8db
Compare
036a8db
to
1326f5a
Compare
@shixiedesign Thank you for the more detailed spec - Updated https://deploy-preview-2848--the-carbon-components.netlify.com/component/file-uploader--default.html upon the update. One question wrt the invalid state is that whether we need an invalid icon there. I'm wondering this because @emyarod originally had it, which can be seen at https://the-carbon-components.netlify.com/component/file-uploader--example-upload-states.html. I have https://deploy-preview-2848--the-carbon-components.netlify.com/component/file-uploader--example-upload-states.html for now. |
@asudoh investigating this right now along with issue #2901 (comment) |
This comment has been minimized.
This comment has been minimized.
Cool thanks @shixiedesign for specing this and entering the issue! |
Superseded by: #3175 |
Closes: #2288
Not for this milestone (
v10.3
), but creating a draft pull request to get designer's input. Notably:v10
doesn't. Was it a remainingv10
item?The latest demo can be seen at: https://deploy-preview-2848--the-carbon-components.netlify.com/component/file-uploader--default.html
Changelog
New
Changed
Removed
Testing / Reviewing
{{ Add descriptions, steps or a checklist for how reviewers can verify this PR works or not }}