design-system-components icon indicating copy to clipboard operation
design-system-components copied to clipboard

[DRAFT] File upload component

Open toxamiz opened this issue 5 years ago • 13 comments

File upload component draft. #613

Contains basic implementation on vanilla module for single file upload and multiple upload with display of selected files.

To do:

  1. Styles. Need some sketches for that component.
  2. Functionality. Public API, other features like auto send files on server, errors etc.
  3. Other stuff

toxamiz avatar Mar 10 '19 13:03 toxamiz

@toxamiz Thanks for tackling this component! It will probably take us a week or so to go over but face value - looking good 👍

azerella avatar Mar 11 '19 23:03 azerella

Hey @toxamiz this is a great first PR!

A few things I picked up:

  1. Any reason why a label is being used instead of say a button for the file upload input selector? I think this does not follow a11y and semantic html. I wouldn't think to click text in a form to trigger the upload file popup. I'd try to do something like this:
<div class="au-file-upload">
  <label class="au-label">Attach drivers license</label>
  <input class="au-file-upload__input" type="file" />
  <button class="au-btn au-btn--secondary">Choose file</button>
  <span class="No file chosen" />
</div>

So it could look something like this in the context of the form:

It could even be worth using au-btn--tertiary for the choose file button. This is because a form page may contain a secondary button for clearing the form or going to a previous page.


  1. I'd hide all the custom styling when javascript is disabled, since it gets in the way of the browser default file input.

image

We add a .no-js class when javascript is not loaded on a page. You can use a selector that goes something like this:

.au-file-upload {

.no-js & {
//insert styles here
}

This is a good example as well: https://github.com/govau/design-system-components/blob/bb844e31a46d388316673533a9ac32d3ded05d12/packages/side-nav/src/sass/_module.scss#L72-L75


  1. There are a couple of problems with the multiple selector: You are using a primary button for the remove file option:

image

This can be confusing, especially if this is contained within a form that has a primary button that submits it.

I would probably remove that functionality to remove altogether and just stick to the default functionality for this, which replaces the previous files selected with the new files selected.

Also we make sure all of our components are supported back to IE8. I'm not sure if multiple file feature is supported in < IE10? If not, do you know of any work arounds?


  1. I would probably even break this PR up. We could release an alpha version, that just contains a single file upload component. And possibly build on it with multiple file upload component, since it's easier to manage that way.

Let me know if this doesn't make sense.

sukhrajghuman avatar Mar 13 '19 06:03 sukhrajghuman

Branch rebase on latest develop now. Added email :)

@adamzerella Do i need to resolve conflicts in auds.json? Or i can simply accept all from develop and then run npm i in package directory?

toxamiz avatar Mar 13 '19 18:03 toxamiz

@sukhrajghuman

  1. Any reason why a label is being used instead of say a button for the file upload input selector? I think this does not follow a11y and even semantic html. I wouldn't think to click a label in a form to trigger the upload file popup. I'd try to do something like this

I'm not sure. Using label is a native way for associating with input, so don't need to call click on input when click on button. I think for a11y it is better using label instead of button in that case. Something like this (i may be completely wrong)

<div class="au-file-upload" id="js-drivers-license">
	<label class="au-label">Attach drivers license</label>
	<input class="au-file-upload__input" id="drivers-license" type="file" />
<!-- hide below when no js -->
	<label class="au-btn au-btn--secondary au-file-upload__label" for="drivers-license">Choose file</label> 
	<span class="No file chosen" />
</div>

2

Yes, thanks, i forgot about hiding elements.

3

Primary button there only for example :) File list functionality may be various: may be we want just see the amount of files we selected like '3 files selected', or may be we want to manage uploaded files like in my PR, or use it as you say, or add an "upload" button on each item in list with upload progress bar, or all of that and switch between options :)

Unfortunately multiple file feature, accept and drag&drop is not supported in < IE10. You can inform user to choose file one by one and add an new input after user select file. It seem like completely separate package specific for that browsers. Also i think u can't access file data because FileAPI is not supported, so cant count size and etc.

4

It depends on functionality, it can be even better to have simple and small single select component and multiple select component with different modes, file list management, upload progress bars, other cool fireworks. And some "core" module between them.

Of course we could release an alpha version just need to decide features.

toxamiz avatar Mar 13 '19 21:03 toxamiz

Branch rebase on latest develop now. Added email :)

@adamzerella Do i need to resolve conflicts in auds.json? Or i can simply accept all from develop and then run npm i in package directory?

@toxamiz I would accept the latest develop and then do: npm run watch in any package to generate the latest auds.json

azerella avatar Mar 13 '19 23:03 azerella

@toxamiz thanks for getting back!

If you can find me an examples/research that supports using a label instead of a button for clicking to upload files, I'd be glad to accept this approach.

Yes lets split this PR up and focus on releasing a file upload component for a single file. I think that would provide good user value and is a lot more simple to build with fewer edge cases. What do you think?

sukhrajghuman avatar Mar 13 '19 23:03 sukhrajghuman

@adamzerella OK, thanks

@sukhrajghuman

If you can find me an examples/research that supports using a label instead of a button for clicking to upload files, I'd be glad to accept this approach.

I don't think there are researches "button vs label on input file". If you want add a caption on form control better use label tag than any "label like" element.

According to this label for form control actually works like button providing a click events to associated form control. Almost any articles about custom styling file input uses styles on label. Buttons uses for other features.

Yes lets split this PR up and focus on releasing a file upload component for a single file. I think that would provide good user value and is a lot more simple to build with fewer edge cases. What do you think?

OK, lets focus on single file upload. So what do we need?

  • Default markup
  • Check file type according to accept attribute
  • File size restriction
  • Error handling/messages
  • ...

toxamiz avatar Mar 14 '19 07:03 toxamiz

@toxamiz Hmmm yea okay I know what you are saying, can we style the associated label look like a button (or link) then. So it doesn't look like this:

image

What do you think?

sukhrajghuman avatar Mar 14 '19 22:03 sukhrajghuman

But yea sorry I probably didn't word myself properly. What I mean to say is that it's not intuitive to click on text to trigger an action. I know labels are used with a for-id relationship to focus on the form control, but you shouldn't rely on that, and you are assuming that when people fill out a form they will click the label. If something requires clicking it needs to be designed like so.

A picture from an article on affordance to better illustrate what I'm trying to say.

image

I would also check out:

https://design-system.service.gov.uk/components/file-upload/ https://home-office-digital-patterns.herokuapp.com/patterns/upload-file https://github.com/alphagov/govuk-design-system-backlog/issues/49

sukhrajghuman avatar Mar 14 '19 23:03 sukhrajghuman

@sukhrajghuman I see the reason of misunderstanding. Actually i meant that label should be look like a button, but for some reason didn't add an au-btn class. :) There is no doubt that if something requires clicking it needs to be designed like so.

I think the main accessibility issue with file upload is that native upload provide it's own button and text. I think the better way is without js it should both label and native input. Only with js it converts into button and visually hidden input + some dynamically added element showing which file uploaded. It's not enough to add some css classes for visual appearance

Something like this: image

toxamiz avatar Mar 15 '19 14:03 toxamiz

@toxamiz yes but that above example could potentially be confusing in the context of a form with a primary submit button. Hence why in my first example I used the au-btn--secondary.

image

It's not an easy problem. There are so many edge cases. I will have to talk with our accessibility team for more guidance.

Right now I'm thinking we just stick with the browser default file upload, and add some additional styling (fonts, spacing, errors, focus states etc.) That way we don't have to worry about javascript either.

sukhrajghuman avatar Mar 18 '19 01:03 sukhrajghuman

I will have to talk with our accessibility team for more guidance.

That would be great.

Right now I'm thinking we just stick with the browser default file upload, and add some additional styling (fonts, spacing, errors, focus states etc.) That way we don't have to worry about javascript either.

Good idea. Is there any sketches?

toxamiz avatar Mar 18 '19 13:03 toxamiz

Good idea. Is there any sketches?

Not at this stage sorry. I was basing this off the gov uk implementation of the file upload component, as mentioned in my previous post. Will keep you in the loop

sukhrajghuman avatar Mar 20 '19 03:03 sukhrajghuman