File Field design update: Upload field.

Marked #1683838: Add HTML5 Drag & Drop upload to Field file as duplicate of this.

clearly missing a tag

...moving related issues to their dedicated metadata section.

Is there any plan to push this forward?
I wonder if there's an issue somewhere to bring the drop file functionality into core by some library or custom solution? This surely needs to be done before any styling :)

But maybe it is too much for the scope of this issue and we should focus only on styling the current file field implementation using those mockups as style quide?

I'm trying to make it work with FileAPI only. The browser support seems OK but please let me know if this will be enough? FileAPI browser support.

Yes I understand that we want to use current behaviour where possible. So the only thing we want to do is adding drag&drop and without using external libraries like dropzone.js.
What I have tested so far is adding a behaviour using only event.dataTransfer property for drag and drop events witch is a part of File API.
The patch does not work though - the files are not being uploaded.

I wonder if we could try and use parts of Drag & Drop Upload module here. Looks like it does it without external libraries and yet using standard Drupal file upload.

@philipz: I was about to post a link to that project but you beat me to it.

@nod_ could you take al ook at it?

Discussed this with @LewisNyman and @G-raph at DrupalCamp Ghent. @G-raph is very interested in taking this on. But @LewisNyman pointed out that the biggest stumbling block is likely going to be the integration with Drupal's AJAX system-based file uploads. Hence I proposed to handle that side of things, if @G-raph could handle the CSS and general JS side of things. Basically, @G-raph would do everything except for the Drupal.ajax stuff.

Hopefully we can get this moving forward again :)

Curious if this ever got moving again :)

Make it look right, I'll make it work right.

I think we can make the JS work fairly easily with the approach in #10.

At this point I think this is 8.1.x material.

Is this great new style available in a contrib module?

Now there should be working dropzone element with working add files button

Kinda sorta works:) Fun!

Limited the functionality to file fields because image fields will be managed in #2115469: Image Field style update.

One big questions I had was how are we going to integrate the multi field widget into this component because it has the drag handle. Anyone has ideas?

:O Great work, Lauri! This is a major usability improvement IMO.

Yay @lauriii for working on this! Is your question in #30 about the code or the ui behaviour?

@yoroy it is about the UI behaviour

It would simply just work ;-)
Do you mean that there's no design for where the draggable thingy should be positioned?
That would need some design work indeed, current state is quite ugly:

So there is a file component but it doesnt help much with the multi value field (with dragging). Just a notice that this issue is presumably mainly focusing on the file upload style instead of the image one which is again completely new story.

Lets move the [x] remove - next to the 4kb and then place the drag and drop at the far right.

I've been working on this guy as an extension of lauriii's patch.

I'm having an issue installing uploadprogress.so for PHP 7.0, has anyone had any luck with this yet? I cannot find any guides on compiling the proper version. It seems like uploadprogress-1.0.3.1.tgz possibly is only supported up to 5.4?

Adding padding to file input is a clever way but we shouldn't do it. For example, IEs won't support it. Any browsers can take it off at anytime since it's not W3C standard.

DragEvent provided more info and more extendable. eg. we can prebuild thumbnails from client side.

I've made some headway on getting this one towards a completed state. I just had a few notes I would love some feedback on;

1. The current design only accounts for an upload progression bar, however you need a 3rd party library (PECL upload progress or APC to be able to view this), in the majority of cases most users will be stuck with the existing throbber. Which currently, we don't have a UX pattern for.

2. Would it make sense to attempt to implement the HTML5 progress tag to replace the current upload progress structure of div's etc?

Thanks!

@AaronChristian: Please post your progress here so you can get incremental reviews and other people can keep working and testing it!

One of the ideas for the throbber design I had was to replace the icon on the left with a throbber.

Hey guys, sorry this took so long. I was having some issues with the patch, so hopefully this has all the changes. If it doesn't work let me know and I'll revisit making it.

Thanks!

+++ b/core/themes/seven/js/dropzone.js
--- a/core/themes/seven/seven.libraries.yml
+++ b/core/themes/seven/seven.libraries.yml

Rather than put this in the Seven theme, wouldn't it make more sense to be in file.libraries.yml?

Similarly, rather than add this to the existing widget, might it make more sense to either provide a new widget for files, or make this a configurable option on the existing widgets?

@AaronChristian, I wrote a blog article on my best practices for generating patches and interdiffs, which might help you in the future: http://openconcept.ca/blog/mparker/best-practices-generating-patches-and...

Awesome thanks @mparker17, will have a read.

Cheers!

triggering the testbot for #44.

Thanks Manjit.Singh, I'm at little confused as to what has failed in the test. Are you able to point me to what the issue in my code is? I've never really worked on submitting and fixing patches before.

Thank you!

@jhedstrom thank you as well. I'll leave that to someone else to comment, I just worked off of what was there originally. However I do like the idea of a toggle setting or something.

@AaronChristian to find out what failed:

1. look in the comment where you submitted your patch
2. under your patch, you'll see one or more buttons that show the version of PHP and MySQL used, and how many failures (in the case of the patch from comment #44, it looks like PHP 5.5 & MySQL 5.5 18,509 pass, 2 fail
3. click on the button. You'll go to a page (in the case of the patch from comment #44, https://www.drupal.org/pift-ci-job/232917) that shows you which test classes and which assertions in those classes failed

If you just scroll through the patch contents, you'll see that the contents are mangled.

I bet it is good but the patch file itself is broken.

@Fidelix that's actually intentional: binary files get included in the diff to make it easier to review (see #1111224: Introduce .gitattributes to end the CRLF/LF and binary diff horror, and detect/auto-fix whitespace errors, #2166013: Remove macros from .gitattributes to avoid Git errors., https://www.drupal.org/node/1054616#comment-4994354 for background).

(previously, binary files didn't show up in patches, and we had to manually download them from the issue queue and place them in the correct place in the repository before reviewing or committing).

If you're having trouble applying the patch, and you're working with a clone of Drupal 8.2.x core, try applying it with git apply. Modern versions of patch(1) should also work; but if you're patching on Windows, you may need to pass --binary (I have not verified this though — see the patch(1) manpage for more information).

Not sure if this fixes things as my simple test on my dev server keep throwing 500 batch errors. But perhaps we can get this tested again.

This status change should help. :)

Retagging for the media initiative's current tag.

Found some stuff -- mostly coding standards violations, but also a couple of bigger concerns...

1. +++ b/core/modules/file/file.module
   @@ -1275,6 +1277,29 @@ function template_preprocess_file_link(&$variables) {
   +/**
   + * Gets a class for the icon for a MIME type.
   + *
   + * @param string $mime_type
   + *   A MIME type.
   + *
   + * @return string
   + *   A class associated with the file.
   + */
   

   This doc comment appears to be completely inaccurate...

2. +++ b/core/modules/file/file.module
   @@ -1275,6 +1277,29 @@ function template_preprocess_file_link(&$variables) {
   +function file_shorten_filename($link_text) {
   +
   +  $name = $link_text;
   

   There's an extra line of white space here, and it seems like the argument should be $filename, not $link_text.

3. +++ b/core/modules/file/file.module
   @@ -1275,6 +1277,29 @@ function template_preprocess_file_link(&$variables) {
   +  $nameWoExt = chop($name, '.' . $ext);
   +
   +  $nameStart = substr($nameWoExt, 0, 10);
   +  $nameEnd = substr($nameWoExt, -4, 4);
   

   This may be nitpicking, but can the variables use a consistent case? Core favors $name_without_ext, not $nameWoExt.

4. +++ b/core/modules/file/src/Element/ManagedFile.php
   @@ -238,7 +238,7 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
   +      '#attributes' => ['class' => ['upload-button js-hide']],
   

   This should probably an array with two elements: ['upload-button', 'js-hide']

5. +++ b/core/themes/seven/css/components/dropzone.css
   @@ -0,0 +1,105 @@
   +.dropzone-buttons li {
   +  display: inline-block;
   +  margin-right: 10px;
   +}
   \ No newline at end of file
   

   Git needs a newline at the end of the file.

6. +++ b/core/themes/seven/css/components/dropzone.css
   --- /dev/null
   +++ b/core/themes/seven/images/dropzone-new.png
   

   I wonder if these should be SVG, if possible, for flexibility's sake? (A side advantage is that the patch will not run afoul of Git's binary handling if it's using SVGs.)

7. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,132 @@
   +  };
   +  ¶
   

   The blank line has extra whitespace that needs to be removed.

8. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,132 @@
   +  if(Drupal.ajax) {
   +
   +    Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
   

   Missing a space between if and (Drupal.ajax), and there's a needless blank line after it. It would also be nice if the setProgressIndicatorThrobber function had a doc comment to explain it.

9. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,132 @@
   +      // move the throbber to the dropzone trigger
   

   s/move/Move

10. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      //if($(this.element).hasClass('upload-button')) {
    +      //  console.log('uploaded');
    +      //} else if($(this.element).hasClass('remove-button')) {
    +      //  console.log('removed');
    +      //}
    

    Commented-out code blocks should be removed.

11. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +
    +    Drupal.Ajax.prototype.success = function (response, status) {
    +
    +      // Remove the progress element.
    

    It seems incredibly dangerous to overwrite the prototype of the AJAX success function. Surely there's got to be a better-encapsulated way of doing this?

12. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      // Save element's ancestors tree so if the element is removed from the dom
    

    s/dom/DOM

13. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      // we can try to refocus one of its parents. Using addBack reverse the
    +      // result array, meaning that index 0 is the highest parent in the hierarchy
    

    Why use addBack() to reverse an array? If memory serves, ES5 has Array.prototype.reverse(). And if we're trying to find the nearest [data-drupal-selector] element, is there any reason not to use $.closest()?

14. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      // If the focus hasn't be changed by the ajax commands, try to refocus the
    

    s/ajax/AJAX

15. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      if($(this.element).hasClass('upload-button')) {
    

    Needs a space after the word if.

16. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +        //console.log('uploaded');
    

    Commented-out debugging lines should be removed.

17. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      } else if($(this.element).hasClass('remove-button')) {
    +        //console.log('removed');
    +      }
    

    This entire else if block can be removed.

18. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      // move the throbber to the dropzone trigger
    

    s/move/Move

19. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +      if($(target).find('.image-preview').length) {
    

    Space after if.

20. +++ b/core/themes/seven/js/dropzone.js
    @@ -0,0 +1,132 @@
    +})(jQuery, Drupal);
    +
    +
    

    Two extra blank lines here.

21. +++ b/core/themes/seven/templates/form-element--managed-file.html.twig
    @@ -0,0 +1,105 @@
    +{#{{ kint(children) }}#}
    

    Should this be here?

Altered a few things in this patch (sorry for the lack of an interdiff, but I think binary data in the patch makes one impossible). And found a couple more things that give me pause...

1. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,132 @@
   +      $dropzone.on('dragover mouseenter', function() {
   +        $(this).parents('.dropzone').find('.dropzone-trigger').addClass('is-hovering');
   +      });
   +
   +      $dropzone.on('dragleave mouseleave', function() {
   +        $(this).parents('.dropzone').find('.dropzone-trigger').removeClass('is-hovering');
   +      });
   

   This looks ripe to use $.on()'s event delegation...

2. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,132 @@
   +      $('.js-upload-image').click(function(e) {
   +        $(this).closest('.js-form-item').find('input[type="file"]').click();
   +        e.preventDefault();
   +        e.stopPropagation();
   +      });
   

   A doc comment explaining this might be useful.

Yoooo! This one oughta pass the tests.

I think a JavaScript maintainer should take a good long look at the JavaScript parts of this patch before it can be RTBCed.

1. +++ b/core/modules/file/file.module
   @@ -1286,6 +1288,31 @@ function template_preprocess_file_link(&$variables) {
   +  $ext = pathinfo($filename, PATHINFO_EXTENSION);
   +
   +  $name = basename($filename, '.' . $ext);
   

   Since we're on PHP 5 > 5.2 we can use:

   $name = pathinfo($filename, PATHINFO_FILENAME); 
   

   Not sure of the need for this though.

2. +++ b/core/modules/file/src/Element/ManagedFile.php
   @@ -238,7 +238,7 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
   +      '#attributes' => ['class' => ['upload-button', 'js-hide']],
   
   @@ -255,6 +255,7 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
   +      '#attributes' => ['class' => ['remove-button']],
   
   +++ b/core/themes/seven/templates/form-element--managed-file.html.twig
   @@ -0,0 +1,105 @@
   +        <ul class="dropzone-buttons">
   
   +++ b/core/themes/seven/templates/image-widget.html.twig
   @@ -0,0 +1,27 @@
   +    <div class="image-preview">
   

   js-hide is special but the rest of classnames that js uses should be data attributes. as in data-drupal-*

3. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,109 @@
   +    attach: function (context, settings) {
   ...
   +    attach: function (context, settings) {
   

   Missing the detach function

4. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,109 @@
   +        $(this).parents('.dropzone').find('.dropzone-trigger').addClass('is-hovering');
   ...
   +        $(this).parents('.dropzone').find('.dropzone-trigger').removeClass('is-hovering');
   

   Why not closest?

5. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,109 @@
   +      $(context).find('.js-upload-image').click(function(e) {
   

   always use .on(), delegation or not. and when triggering an event, always use .trigger() when dealing with jquery objects.

6. +++ b/core/themes/seven/js/dropzone.js
   @@ -0,0 +1,109 @@
   +    Drupal.Ajax.prototype.setProgressIndicatorThrobber = function () {
   ...
   +    Drupal.Ajax.prototype.success = function (response, status) {
   

   Ok, we can't do that in a theme. Maybe we need to move things around in ajax.js so that it's easily overridable in themes

   As far as I understand this it to change the place where the indicator is appended. Almost seems like we should create a new progress indicator type.

   The lesser evil would be to go through Drupal.ajax.instances and replace the success and indicator callbacks for the file upload field only, not everyone.

Some other issues: when I add a file and remove it. The next time I try to upload a file I get the file selector dialog twice, some attach/detach thing not going on. Maybe because the attach behaviors are missing .once() after element selectors.

I wouldn't necessarily delegate the mousz*/drag* event, since It's more costly to run it on the whole form than just the zone we're interested in.

When biding/unbinding event always add a namespace. here it could be dragleave.file-upload mouseleave.file-upload or something.

There are a few debug lines that needs to be removed too.

It's close, great stuff :) the handling of the progress indicator need some work to be cleaner.

Thank you, @nod_, for the review! I didn't address any of your comments in this patch because a) I don't quite grok the JS stuff it's trying to do, and b) I just want the damn tests to pass :)

I hate it when I say "this will pass the tests", and it epically fails. So I'm not going to say this one will pass.

Made a few changes to the JavaScript -- mostly trying to get rid of the dodgy method overrides in Drupal.Ajax.prototype. No interdiff due to binary data.

Other than some spacing snafus (once a file is uploaded, the elements are too close to the borders of the file field), this looks pretty good. Attached some video proof :) The "hover" state for the file dropzone is also not styled as designed. Those are the only things I found visually.

Also the progress bar and remove elements are not implemented exactly as designed, but there was no design for a file field with extra elements either.

Tried to address the padding, made the button smaller(although the delete button could use a bunch more styling and a Seven preprocess hook)

Not sure how to deal with the preview image, it doesn't make sense to add it to the drop area because the size changes...

It looks like we've had the javascript maintainer review from @nod.

Overall it's very difficult to see from recent comments what work is still needed on this, hence needs IS update to clarify?

Just 2 cents.

I think it doesn't fulfill the IS and frontend code styles. We have to define a better HTML & CSS for it. e.g.:

1. Define a pattern.

Check it out the IS, we have 3 states: pre-upload -> uploading -> uploaded

<div class="dropzone">
    <div class="dropzone__preview">
        <!-- FILE FIELD -->
    </div>

    <div class="dropzone__container">
        <div class="dropzone__pre-upload">
            <div class="dropzone__actions">
                <button class="dropzone__add-files">Add File</button>
                <button class="dropzone__browse-libary">Browse Library</button>
                <button class="dropzone__other-button">Dropbox (Module Custom)</button>
            </div>
            <div class="dropzone__description"></div>
        </div>
        <div class="dropzone__progress is-uploading">
            <button class="dropzone__close">Close</button>
            <div class="dropzone__progress-filename">Uploading file-name</div>
            <div class="dropzone__progress-bar">
                <bar />
            </div>
            <div class="dropzone__progress-stat">Uploaded 2.6 of 4KB</div>
        </div>
        <div class="dropbox__uploaded">
            <div class="dropbox__uploaded-filename">interdiff--17.txt</div>
            <div class="dropbox__uploaded-stat">4KB</div>
            <button class="dropbox__uploaded-close">Close</button>
        </div>
    </div>
</div>

2. Sign off above pattern from frontend maintainers

3. Code & Add CSS3 Anime with basic JS

4. After real coding, sign off again from frontend maintainers, including the docs comments.

5. Sign off from UX/UE designers

6. Starting advanced JS work. (Pick a lib? Code our lib? Cross browsers testing and fixing..etc)

7. Sign off from JS maintainers

8. Happy Ending.

This is coming up in the context of the Media library design at #2796001: [prototype] Create design for a Media Library. Adding to the current sprint; this looks incredibly close, and would be great to get it finished up for 8.3.

1. +++ b/core/themes/seven/images/dropzone-new.png
   @@ -0,0 +1,219 @@
   +�PNG
   

   Looks like png code has been added here. There's two problems here; one, the image needs to be an SVG (for retina displays); and two, the image needs to be located in core/misc/icons in whichever directory matches it's hex color.

2. +++ b/core/themes/seven/templates/form-element--managed-file.html.twig
   @@ -0,0 +1,105 @@
   +    <span class="field-prefix">{{ prefix }}</span>
   +  {% endif %}
   +  <div class="dropzone js-dropzone">
   +    <div class="dropzone-trigger"></div>
   +
   +    {{ children }}
   

   When I test this in the browser the image is loading into the same div as the upload button, not the area that contains the 'plus' image. I *think* the problem is in the nesting. Like the dropzone trigger div should wrap {{ children }}?

The rest of it is looking pretty good. I think it's getting close to done.

Very excited about this. Would this be novice changes?

I have tested the latest patch against the media patch #151 here: https://www.drupal.org/node/2831274
as part of the Berlin media Sprint.

From what I see, it works already great with single file upload:

But for multiple file upload it still needs some work:
- the icon is missing for the uploaded files
- instead the icon is changed on the input for new file:

I like this improvement.

would be great to get this in 8.3
looks only svg and dropzone issues left

Here is the image with the svg icons. I didn't find another example of an svg using sprites in core, should we pick one of the colors from the image to decide which folder use?

Also I found a small javascript console error when trying this patch (Uncaught TypeError: instance.element.hasClass is not a function), should be fixed now.

I couldn't fix #2 from #80.

NW for #88, #89 and #80.2

#87: It seems this patch lost a couple files as compared with #79 :)

What do you mean? The patch file size is significantly smaller because there is no PNG there.

Tested this again. The lack of padding in the single file scenario that I identified in #69 is still an issue. There was not design for image file uploads as to where the image thumbnail would be placed, so I think its fine for this patch to keep it in the box, but not without a padding :) Here is an up to date screenshot with the latest patch:

The multifile scenario does not look like at all as designed, the uploaded file does not show up as a small bordered item, but I think we can go step by step on this and ALSO the image variant of this was not designed and IMHO it would be sad to wait for that to happen for any improvement:

Where this is used as the default file widget (field settings), the Add files button is still displayed but of course does not work anymore. (The same button is removed when interacting with this widget on the entity form itself, so this does not appear there):

Finally the button says "Add files" even if the field is limited to one file. This problem applies to both the field settings and the entity edit form itself.

Working on this issue on global sprint weekend.

Worked on this in pairprogramming with @katzilla. Current patch implements most parts of the given design. There are still some responsive issues in draggable table on multi value fields. The designed progressbar is not possible to implement at the moment due to a not functional implementation of uploadprogress for php7.

Added some screenshots to give an impression of the developent in #98.

Single file upload:

Multiple file upload:

Multiple images:

Multiple files:

An interdiff for #98 would help reviewers.

Here is the interdiff for #98.

+++ b/core/modules/file/src/Element/ManagedFile.php
@@ -181,11 +181,11 @@ public static function uploadAjaxCallback(&$form, FormStateInterface &$form_stat
+      $form[$current_file_count]['#attributes']['class'][] = 'ajax-new-content managed-file';

I think this should be:

$form[$current_file_count]['#attributes']['class'][] = 'ajax-new-content';
$form[$current_file_count]['#attributes']['class'][] = 'managed-file';

You are right. The classes should be implemented as array values. I am going to upload a modified patch tomorrow. Thanks for your review so far.

Here is the patch from #98 including the remarks from #102.

@Daniel_Rempe

This is coming together nicely. Just a couple of minor things I noticed.

1. The jquery ajax throbber should really appear *inside* the area where the drop icon is and not *push* the button and text over as it currently does.

See quicktime movie: https://www.drupal.org/files/issues/jquery-throbber.mov

On multiple file upload the asterisk indicating tabledrag has happened is reflowing oddly.

Hey all, I have a patch that will fix the issues noted in comments #105 & #106.

Will post tomorrow morning once I get back into work.

Thanks!

Here she be.

Should note: I've added RTL support as well.

Correct:

➜  /tmp  diff file_field_design_update-2113931-104.patch file_field_design_update-2113931-108.patch
➜  /tmp  

Ahhhh crap, I attached the wrong patch file, not sure what happened there. This ones legit though.

Ahh i believe this is failing due to the tabledrag changed (*) being moved within the handler instead of the cell (@tkoleary - fixes the reflowing of the asterik).

I'll have a deeper look into this.

Okay think I found the issue.

One thing that still needs some attention is the default image settings page. That form has a bit of a different layout, wondering if we can force the same sort of layout as we have on a single image upload field.

@ AaronChristian

Lots of improvement here. As noted in UX meeting the only things I saw in simplytest were:

1. The button only works first time, but the plus icon works all the time
2. When the throbber appears it is on top of the icon, instead the icon should be hidden when the throbber is there.
3. In narrower screens the description field overflows the parent containter

@AaronChristian

One more thing. When adding with multi value, for some reason it goes back to the other icon on the third upload. Weird.

@katzilla and I worked on this a little bit more.
We removed the background image when the throbber is shown and realigned the alt and title input fields.

@Daniel_Rempe

Looks much better.

Still have the wrong icon on multi-value fields though.

Fixed the coding standard issues and proposed a solution for the table on mobile. In some cases (when the display option of the file-field is activated, or when 'show row weights' is active on multi-value fields) there is too much content in the table, so it gets messy. We added an overflow, so that one can scroll the table.
We also fixed the issue with the wrong icon on multi-value-fields. Please test and provide feedback.

As we just discussed with @ifrik we should open a separate issue for all mobile display problems and try to get this in for 8.4 since this is already a long list of comments. We opened the issue here https://www.drupal.org/node/2863808

Review points made since #98 that have not been explcitly mentioned as having been addressed:
#116 default image settings page
#118.1

I've updated the IS to reflect
#98: "The designed progressbar is not possible to implement at the moment due to a not functional implementation of uploadprogress for php7." (and created a follow-up #2863846: File Field design update progress bar)
#128 "separate issue for all mobile display problems" #2863808: File Field design update mobile specific table issue

If anyone's looking for something to work on at the DrupalCon sprint, I'd recommend this issue. Won't be there myself though. (I'd work on this myself, but I'm best kept away from anything front-end related.)

Here's a review of where things stand- I looked at a clean install of 8.4 for the "before" screenshot, then applied the most recent patch (#126) for the "after" screenshots. Further details below are outstanding issues from ~#106 onwards. Hope this is helpful!

Before:

After:

Hover:

Loading:

Uploaded:

#116, the remove button is next to the uploaded image thumbnail for the default image field settings page but not for a file upload field on an Add Content page. Also, the layout should be updated so the alt and title fields are *inside* the box with the uploader.
#118, throbber still appears next to icon on default image field settings page rather than on the icon

Also attch, this is what happens when you try to upload an image to an unlimited image field that already has a default image/file defined... this seems confusing, maybe the default image could be labeled? should it appear there?

Awesome thank you so much Elli. I'll review all your feedback as soon as I get back to my work place.

Safe travels home.

I can see alternative text is mandatory now , so could be possible to place filename as default alternative text ? this could save some time.

@nithinkolekar Using the filename as alternative text is an antipattern. WordPress actually removed this functionality late last year for accessibility reasons.

Other references:

• 13.8 How to specify alternate text, WCAG
• An alt Decision Tree, WAI
• WCAG Rule 27: Image file name is not valid alt text, Open Ajax Accessibility Alliance

Did #123 get a fix? It looks like all other followups are filed.

@rootwork Thanks, that's correct - we won't do that and/or introduce that in this patch.

What is left for RTBC?

@Bojhan see #131

I will finish this up.

Is there an image icon we could use rather then a file icon, for images?

@dubcanada, at this time, no. Just due to the flexibility of the file field being able to accommodate different types, not just images and files. Contrib can add different types of widgets to the file field.

The whole multiple files/images needs to be rethought I'm going to take a few days and play around with it and see what I can come up with. It doesn't make much sense right now.

Given we added Media to 8.4.0, and it's still not visible in the UI until the whack of issues under "Required before the module is shown through the UI" is done: #2825215: Media initiative: Roadmap I think it's a good idea to minimize the amount of change happening in the File/Image modules. Therefore, postponing this on the other issue, per the UX team.

Just a reroll for 8.6. I'm not sure if this should really be postponed for the media module. The media module uses file/image fields so it would benefit from this change as well.

I suspect this is about #2831940: Create file field widget on top of media entity not having a moving target.

After testing in Firefox Win/58 and Chrome Win/64 I found a number of additional issues.

1) When I applied the latest patch, the script inclusion line ('js/dropzone.js: {}', seven.libraries.yml in the Seven theme folder) received one level of indentation too much, so that Drupal thought it was a stylesheet. I haven't seen anyone else complain about this, so this might just be my problem.

2) Field labels are absent. This is most noticeable on fields for uploading single images/files, because for fields for multiple/unlimited images/files the summary serves as a sort of label.

This behaviour cannot be influenced through the Manage Form Display tab of the content type.

Presumably this should be fixed in core/themes/seven/templates/form-element--managed-file.html.twig.

Is there any reason why labels were left out of the template?

How is this handled for other types of fields? Is the general implementation that the label is derived from the label sub-field, or does this differ per field?

3) When uploading multiple files/images of which one has the wrong extension, a completely different interface is shown for the valid files. (See illustrations.)

If you select and then delete one of the valid files/images, the system deletes all of the files/images from that particular upload, after which the new upload interface is restored.

Illustration: one valid upload and one invalid upload, pdf1.pdf in a field that doesn't allow PDFs.

Illustration: for contrast, an upload with only valid file name extensions.

4) Alt and title fields for single images break out of their container on narrow screens. (See illustration.)

This is related to the bug reported in #118.3, which was reported fixed in #126.

5) The current dropzone implementation does not degrade gracefully when Javascript is disabled for whichever reason. (See illustration.)

If a user has Javascript disabled, instead of falling back to the default browser interface (which works fine), the dropzone partially replaces that interface, leading to problems:

a. The dropzone is still shown, but should be hidden.

b. The Upload button is shown (correctly), but does not work.

c. A fake Add Files button is shown, but doesn't work.

Part of the problem is that some of the HTML necessary for creating the dropzone is generated through the template rather than using Javascript. Instead, we should use the additional technology (Javascript) to generate and add (to the DOM) the HTML that is required by the additional technology. We could use data attributes to pass information from the CMS to the additional technology.

999) [notabug] Furthermore, in reply to #129, I cannot replicate the issue from #118.1 ("The button only works first time, but the plus icon works all the time."), so I assume it has been fixed. I haven't ran any tests though (I wouldn't know which specific tests to run), I just tried it out in two browsers. I could click the Add Files button multiple times and it worked every time.

This fixes the following issues for the default image in the field settings edit screen (comment #131):
- correct styling remove button.
- correct styling throbber for remove button.
- correct styling upload throbber.

This makes sure alt and title fields for single images do not break out of their container on narrow screens (comment #147.4). (UPDATE: fixed comment number)

https://www.drupal.org/files/issues/2113931-148.patch

https://www.drupal.org/files/issues/interdiff-145-148_2.txt

Great, so if I'm understanding right we now have outstanding:

#131

"Also attch, this is what happens when you try to upload an image to an unlimited image field that already has a default image/file defined... this seems confusing, maybe the default image could be labeled? should it appear there?"

and
#147.1, #147.2, #147.3, #147.5.

(#148 is wrong when it say it addresses #147.2 it means #147.4)

Since @webchick postponed this a year ago, a number of things have changed. The experimental Media Library module provides a vastly improved UI that does not interfere with the work in this issue and would in fact benefit from it. So let's unblock this and continue.

Patch rerolled

I don't understand how to reply testing problems for patch #154, in local there are another testing problems.

Attached is a new patch implementing the following changes:

• Fixed all styling issues:
  • RTL
  • IE11
  • Image field
  • Table view
• Added progressive enhancement via JS instead of through template files. When you don't have JS enabled the regular file field is shown.
• Refactored CSS since the HTML was slightly changed and some of the styling improvements were not directly related to the dropzone. The managed file improvements are in a separate CSS file.
• Added ES6 file for the JS.

There are still some mobile responsive issues for the table. We have #2863808: File Field design update mobile specific table issue for that.

Please review.

Adding screenshots for the latest patch to the IS for easier review.

This should fix the test.

Ahhh, remove some CSS that shouldn't be there. Sorry about that!

Good busy.

"default-image-uploaded-no-js.png"

This screenshot appears to be empty.

Tested Firefox/Win 64.0

Review of patch 162.

a. Should the remove button activate on right-click? It does now. Is this even a problem for this issue? Mouse button handling for removing files appears to be taken care of elsewhere.

b. The shorten filename function lengthens some filenames and also sometimes repeats characters.

Examples (top becomes bottom):

1234567890abcdef
          ^     ^
1234567890abcdef

1234567890abcdefgh
          ^
1234567890...efgh

1234567890abc
          ^
1234567890...0abc

Patch attached.

c. The Unicode patch from comment #91 does not seem to have made it into actual patches.

d. The description/help list can be added to in the content type settings and by using hooks. I have added a few screenshots of how it looks like when a largish text is added.

Thanks Branko! Let's fix it.

A. The buttons works with the jQuery mousedown event as do all AJAX buttons. I think fixing this is out of scope for this issue.
B./C. Nice find. The unicode methods are deprecated in favor of mb_strlen and mb_substr, but I did adopt the proposed changes from #91 since I think that method seems to be a better implementation. Updated the failing test accordingly.
D. Fixed. Had to add some JS to pull the preview image for the single image widget to another wrapper to make it work. The HTML is not optimal for this specific case. Changing it through a template changes the HTML for all other cases as well. This felt like the lesser evil.

Please review!

I came across the following issues in patch 166:

1) A single uploaded file displays a cropped icon if the managed form item only contains the file name and no additional fields. (See illustration.)

2) The title of the Display column in a managed file table is cropped in some browsers and may also get cropped when translated. My solution is to not assume a width for that column. (See illustration.)

3) On very narrow screens, forms.css adds a top margin to the Remove button.

4) The Remove button is positioned slightly too high. This is only really noticeable in the managed form element for a single uploaded file. (See illustration.)

5) If I add a line-break in the Help text field of a field's definintion (/admin/structure/types etc.), this gets displayed as the HTML character entity reference for carriage return in the node edit form's description. I assume this is a problem outside the scope of the current issue. (See illustration.)

6) When I upload too many images (for example 6) to a Limited image field with 5 slots, only five images are uploaded and I get an error message: "Field MACHINE_READABLE_NAME can only hold 5 values but there were 6 uploaded. The following files have been omitted as a result: NNN.ext." This field will be available to users who may not recognise the machine readable name. Should this perhaps be the human readable name?

7) When I upload too many images to a Limited image field with 1 slot, the upload fails without an error message. Should this not work the same as a limited field with multiple values, i.e. upload what you can and give an error message for the rest?

Then there is one minor issue that is a hill I do not wish to die on, but that I wanted to mention nevertheless:

8) The file name shortener still lengthens file names. However, with a proportional font this is barely noticeable because the default ellipsis (three dots) is narrow. Also, it does exactly what it says in the comment ("The maximum number of characters to show from the original string"), so in that sense the function works as advertised.

It is just a bit odd that the function is called file_shorten_filename() when it sometimes lengthens filenames.

Examples for the default limit (10 characters):

1234567890a.txt (base name $limit + 1 characters long)
1234567890ab.txt (base name $limit + 2 characters long)
1234567890abc.txt (base name $limit + 3 characters long)

These become:

12345...7890a.txt (new base name $limit + 3 characters long)
12345...890ab.txt (new base name $limit + 3 characters long)
12345...90abc.txt (new base name $limit + 3 characters long)

The attached patch solves issues #167.1, #167.2, #167.3 and #167.4.

Hm, I was sure I had added all files. Anyway, here are the three illustrations (two issues share a screenshot) plus the patch.

The related interdiff is in #167.