carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.73k stars 1.79k forks source link

FileUploader component uses incorrect icon for file types that are not allowed #5727

Closed modean closed 1 year ago

modean commented 4 years ago

During DnD operations files that are not allowed have a green plus overlay icon instead of the not-allowed overlay icon

Environment

Operating system: macos Mojave

Browser: Chrome 80.0.3987.149 (Official Build) (64-bit)

Detailed description

What version of the Carbon Design System are you using? "carbon-components-react": "7.9.3"

What did you expect to happen? The overlay icon used during the DnD operation should be a red cross or similar and not a green plus icon. The latter indicates that generally speaking the DnD operation can be carried out. Where the red cross indicates that the file type used during the DnD operation won't be accepted by the drop area.

What happened instead? Wrong overlay icon was exposed.

Note: As a reference the native file chooser widget can be used that you get when you click on the drop area. Due to accept being set to {[".txt"]} the file chooser widget of the user agent (browser) would not allow you to select .png files. Instead these files are greyed out. Thus the behaviour of the DnD operation and there its overlay icon should be symmetric. That is it also should indicate what is possible and what not by using the appropriate dropEffect and thus the appropriate overlay icon.

Steps to reproduce the issue

  1. Create a DnD area using FileUploaderDropContainer
  2. Set the value of accept to {[".txt"]}
  3. DnD an image file (e.g. a PNG) onto the drop area
  4. A green plus icon is shown

Additional information

Original Slack Convo:

Good morning, I am using the FileUploaderDropContainer component and I did set the accept property to [".ipynb"] . When I DnD over the drop zone I still see the green plus icon in Chrome despite DnD-ing a PDF file (see animated GIF). I did have a quick look at the implementation and it looks like the onDragOver event does not do any validation. Shouldn’t the onDragOver event handler be doing validation against the accept array of file endings or mime-types and turn the dataTransfer.dropEffect into none when the file does not match what is provided in the accept array?

Or is it intended that the consumer of the FileUploaderDropContainer component has to do the validation after the file has been dropped and thus no validation is done during the DnD operation?

Note: In the GIF you don’t see the green plus icon cause it is some sort of overlay and thus isn’t captured by the GIF capturing app.

accept-not-working

See also:

https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/components/FileUploader/FileUploaderDropContainer.js#L78

https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

https://watsondataplatform.slack.com/archives/C2K6RFJ1G/p1585042290082800

emyarod commented 4 years ago

currently the validation occurs on the drop event

cc @carbon-design-system/design what should the UX look like if the uploader has multiple file uploads enabled and the user tries to drag a group of files which contains an unaccepted file type? should the entire operation be blocked? currently we allow only the valid files through and reject the invalid files, and we do not make any changes to the cursor

laurenmrice commented 4 years ago

I think we should allow all files to be passed through even if they are going to load as invalid. (even if its a group of files at a time). I think this is better communication to the user what is happening to their file, rather than blocking it from showing up. Blocking it can seem like some kind of bug happened with the file uploader itself.

modean commented 4 years ago

@laurenmrice

[...] rather than blocking it from showing up [...]

I agree on one hand but on the other hand seeing the green plus icons for files that are not accepted instead of the not allowed icon at least to me was equally confusing. What I do not know is whether peeps are commonly used to this pattern of an overlay icon that provides a visual cue to the user or not.

I also have to admit that I did implement DnD patterns on my own in the past so I might be more familiar with these overlay icons than the regular end user.

modean commented 4 years ago

@laurenmrice

[...] rather than blocking it from showing up [...]

Also as mentioned above in one of my remarks, my main intent was to make the DnD interaction pattern consistent with the behaviour of the file chooser interaction pattern.

There if you click on the dropzone instead of dropping a file you get the native file chooser as exposed by the user agent (browser). Due to the accept property being set file types that are not allowed are greyed out. The user would not be able to select them and thus the user does not get any feedback on what is going on.

Thus in our product we decided (and I as well saw that in the Carbon guideline) to always put an explanatory tagline atop of the dropzone that calls out the file limitations that are imposed onto the user by the dropzone. That way the user can make an informed decision and would know why he/she would get the not allowed icon or why files in the native file chooser widget are greyed out.

After all I can also imagine that the native file chooser widget is the interaction pattern that is the preferred interaction pattern but I guess that requires user research (I do not think we instrumented our product so that we would have metrics about which interaction pattern is more frequently used).

image