connectivedx / Phoenix

http://connectivedx.github.io/Phoenix/
33 stars 5 forks source link

File upload improvements #155

Open kamsar opened 8 years ago

kamsar commented 8 years ago

The default file upload configuration expects custom file upload markup, e.g.:

 <button class="custom-file-upload" type="button">Select file</button>
 <input type="file" name="Media"/>

This is a bit confusing to have this unwritten markup convention (and it also display:nones bare input type=file). It also gives zero indication to the user that they've selected a file.

I propose two modifications:

1) Add an additional tag to store the currently selected file name, if any. I've implemented this on a project thusly:

HTML:

<button class="custom-file-upload" type="button">Select file</button>
<span class="filename js-hidden"></span>
<input type="file" name="Media"/>

JS:

$('.custom-file-upload').click(function(e) {
e.preventDefault();

$(this).nextAll('input[type="file"]')
        .click()
        .change(function() {
            var val = $(this).val();
            if(val.length > 0) {
                $(this).prev('.filename').html(val).show();
            }
        });
  });

And secondly, to add some JS to automatically wrap input type=file with the expected markup, so that you do not need to know the details to successfully markup a file upload.

Lastly, does this code really belong in a file called documentReady.js? I mean, I know it's most performant to have one ready function, but it's terrible separation of concerns.

stoff commented 8 years ago

this is cogent. not sure why exactly the current version does not show the filename; I know I (and others) have written versions of this that do show the filename. One thing to note is that $("input[type=file]").val() will only return the filename. the native control will show the path as well; in cases where the client desires a custom look AND the path, this pattern becomes much more complicated.

I agree that this should be removed from documentReady.js. it should be a plugin and in its own file. I think the general approach you've outlined is good, but should be tuned a bit to work in a plugin. I'll make a branch and share an example.

elseloop commented 8 years ago

In addition to all of the good points above, I think the fact that native input[type="file"]s are display:none in our default stylesheets is a bug we should push to remedy sooner that later. Removing that from the stylesheets seems a good candidate for a hotfix branch/stand-alone dot release.

The JS work around this (I like @stoff's outlined approach) can work alongside any work in #154. Getting rid of any actual DOM calls in documentReady.js (or just getting rid of documentReady.js altogether, because really) is definitely on the list there.

stoff commented 8 years ago

Point of fact - the control is not display: none; the button is set to display: none for html.no-js but the input[type="file'] gets visibility:hidden for html.js. Sass is here: https://github.com/connectivedx/Phoenix/blob/master/Assets/src/css/base/defaults/_forms.scss#L193.

I know the last time I worked on one of these display:none made the input unclickable, but at that time you also had to put the button on top of the input in order to support older browsers. I suspect that display: none may still disable the input, though, so don't do that, people.

elseloop commented 8 years ago

A nice resource on this subject: “Styling & Customizing File Inputs the Smart Way” on Codrops.

ajmueller commented 8 years ago

:+1: for that Codrops bit. Slick stuff.