When some Ajax request occurs in same form (page) with DropzoneJS widget -> selection is cleared in drop-zone. Problem is that DropzoneJS instance is destroyed and with that current files in list (accepted and rejected) are gone.

Issue is noticed with implementation of following issue: #2823670: Improved MultiStep selection display (DropZone Widget).
And problem is introduced with following issue: #2760175: Uncaught Error: Dropzone already attached..

CommentFileSizeAuthor
#22 2827120_22.patch2.6 KBmtodor
#22 2827120_interdiff_22_17.txt1.34 KBmtodor
#17 2827120_interdiff_17_13.txt761 bytesmtodor
#17 2827120_17.patch1.85 KBmtodor
#13 dropznejs_is_cleared_on-2827120-13.patch2.01 KBchr.fritsch
#11 interdiff-2827120-10-11.txt1.25 KBchr.fritsch
#11 dropznejs_is_cleared_on-2827120-11.patch9.06 KBchr.fritsch
#10 interdiff-2827120-9-10.txt1.39 KBchr.fritsch
#10 dropznejs_is_cleared_on-2827120-10.patch9.01 KBchr.fritsch
#9 dropznejs_is_cleared_on-2827120-9.patch8.59 KBchr.fritsch
#8 dropznejs_is_cleared_on-2827120-8.patch8.13 KBchr.fritsch
#7 dropznejs_is_cleared_on-2827120-7.patch6.33 KBchr.fritsch
#2 2827120_2.patch1.31 KBmtodor

Comments

mtodor created an issue. See original summary.

mtodor’s picture

mtodor commented
StatusFileSize
new 2827120_2.patch1.31 KB

Integration should be refactored, but here is quick simple solution. With usage of "once()".

mtodor’s picture

mtodor commented
Status: Active » Needs review
mtodor’s picture

mtodor commented
Assigned: mtodor » Unassigned
primsi’s picture

primsi commented
Status: Needs review » Needs work

Thanks for the work. I've tested this a bit. Everything works fine except the media entity ief widget:
1. when I drop a file and the ief forms are generated, the file doesn't get removed from dropzonejs
2. drop a second file in dropzonejs, nothing happens

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

We have discovered this, too. We think the correct solution for the ief widget should be:

For multi-value fields:

  • Drop a file: form appears, file stays in dropzone
  • Drop a second file: second form appears in dropzone, both files stay in dropzone
  • Remove one file form dropzone: The corresponding form disappears

For single value fields:

  • Drop a file: form appears, file stays in dropzone
  • You are not able to add a new file until you have removed the first one
  • Remove one file form dropzone: The corresponding form disappears

I guess we should handle this in an other ticket.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new dropznejs_is_cleared_on-2827120-7.patch6.33 KB

New patch implements the proposed solution from comment #6.

Also a fix for #2825429: Dropzonejs in EB multi-step selection creates multiple entities is included, because i had to change the getFiles method again.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new dropznejs_is_cleared_on-2827120-8.patch8.13 KB

I refactored the patch a bit and fixed an issue we had with tabs.

If you were on Dropzone IEF tab and uploaded an image, then switching to another tab in eb, and then switching back, the form was broken. IEF form was there but not the file into the dropzone

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new dropznejs_is_cleared_on-2827120-9.patch8.59 KB

Fixed an issue we had if you drop the same image twice. Also i fixed coding style issues.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new dropznejs_is_cleared_on-2827120-10.patch9.01 KB
new interdiff-2827120-9-10.txt1.39 KB

I'm not sure about this change.

In Thunder we have implemented a hook_file_create which could change the filename.

So to make this functionality work, i have to create a dummy file to get the actually filename.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new dropznejs_is_cleared_on-2827120-11.patch9.06 KB
new interdiff-2827120-10-11.txt1.25 KB

Added the uuid into formstate to prevent issues if there are more then one dropzones in an entity browser

primsi’s picture

primsi commented

Sorry for the late reply,.... winter is coming and it's armed with flu and cold.

Regarding the issue from #6. The question here is what do we want from dropzonjs drop-area:

  1. To mirror what is happening in the uploading/editing process of the entity browser workflow. Ie. if we remove a file from the select list, then same thing happens in the drop area
  2. To be only a drop-area that cares only about constraints at drop time (file size, image size, max files) and doesn't care what happens after that. This means that it's ok if we empty the area each time we rebuild the form and we find a way to address cases when constraints were violated.

Until now DropzoneJS was treated as 2. And IMHO we should keep it this way:

  1. we will avoid adding complexity that will come with trying to keep both sides in sync
  2. it will be easier to maintain consistency of UX across implementations (widgets, element implementations,...)
  3. we have less control and ability to implement changes upstream than we have on the Drupal side if needed

Given my slower response here I will not nit-pick about this, but in further issues, if additional problems (not related to the one the the issue deals with) are discovered the way to handle this should be to open a new issue and provide a patch there. If we pile multiple issues in one patch it will be very difficult later on to navigate through what exactly happened where and why.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new dropznejs_is_cleared_on-2827120-13.patch2.01 KB

Ok, going a few steps back.

The new patch addressed only to your comments from #5

The behavior of dropzone stays now as before.

primsi’s picture

primsi commented
Status: Needs review » Needs work

Thanks for working on that. I've tested it a bit today. The only issue I found was:

1. Drop 2 files in the ief widget where one was too big,
2. The one that is the allowed size gets removed and the one which is too big remains marked as rejected

With the other widgets the behaviour is: both files remain in the dropzone but the rejected one is marked as rejected. Which s what we want IMHO.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Issue tags: +dcmuc16
mtodor’s picture

mtodor commented
Assigned: Unassigned » mtodor
mtodor’s picture

mtodor commented
Assigned: mtodor » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2827120_17.patch1.85 KB
new 2827120_interdiff_17_13.txt761 bytes

I have checked IEF functionality a bit. Currently there is this part of code in "modules/eb_widget/js/dropzonejs_eb_widget.ief_edit.js":

          if ($form.hasClass("dropzonejs-disable-submit")) {
            var $submit = $form.find('.is-entity-browser-submit');
            $submit.prop("disabled", false);

            item.instance.on("queuecomplete", function () {
              var $form = this;
              $('#edit-edit', $form).trigger('mousedown');
            }.bind($form));
          }

What it does is -> when ever file is dropped (or multiple of them), form is submitted over Ajax after all uploads are finished. Without changes provided in #2, that would clear DropzoneJS.
So, all we have to do is -> clear all files in drop zone, instead only valid files.

Here is patch and interdiff.

primsi’s picture

primsi commented
Status: Needs review » Needs work

Yes, the ief widget "clicks" on the submit button when queuecomplete.

So, all we have to do is -> clear all files in drop zone, instead only valid files.

Why not check in the ief widget if getRejectedFiles has something and if yes, don't trigger the mousedown? If we do it like that we have a consistent behaviour across all the widgets.

On the other hand, won't we have the same problem in #2823670: Improved MultiStep selection display (DropZone Widget)? I din't check the patch in detail yet, but we will still have to deal with "what to do if some files are rejected".

If I remember correctly we were talking about two options in IRC:

  • if it's possible: accepted files are auto submitted, rejected remain in dropzone
  • if the above it's not possible: don't auto submit at all

So, all we have to do is -> clear all files in drop zone, instead only valid files.

As discussed and agreed we want to avoid silently remove files if something wen't wrong.

mtodor’s picture

mtodor commented

@Primsi this ticket is not about solving IEF issues, it's about fixing bug introduced in #2760175: Uncaught Error: Dropzone already attached.. It's about making dropzonejs works properly in environment with ajax calls and need for that will happen sooner or later. Also future functionalities can prosper by using ajax.

What was my plan with this ticket (but it didn't go in that direction):

  1. make minimum changes and least possible impact on current functionality (that's tricky because fundamental functionality is changed)
  2. after that, address issues or improving of functionalities in other tickets

Also I think that:

Why not check in the ief widget if getRejectedFiles has something and if yes, don't trigger the mousedown? If we do it like that we have a consistent behaviour across all the widgets.

Is not scope of this issue. It's related to IEF widget independently of problem we want to fix here (that issue exists also with current code base).

When it comes to #2823670: Improved MultiStep selection display (DropZone Widget), it's solved as it's visible on animated gif (first option you have listed: "accepted files are auto submitted, rejected remain in dropzone").

And to be honest, I think it would be nice to have similar functionality in IEF widget too. When you have 3 valid files and 2 invalid, that after drop you get 3 forms and 2 files stay marked as invalid in drop zone and after that you can focus on things that are important for you (filling forms for valid files) => Instead of: removing failed files, until you get all valid files in dropzone. And suddenly generation of form is triggered. Also you have to think about cases, what will happen when you drop additional invalid files if you already have all valid in dropzone, etc. I think all that are topics for other tickets.

primsi’s picture

primsi commented

Yes I agree, this issue got derailed pretty badly. We should have stopped sooner :) We will try to do better in other issues.

I agree with most of your points in #19 and yes, this should be topic for other issues.

But anyhow, back to the point, unless I am missing something, the latest submitted patch removes all files from ief widget if one (or more) of those are bigger then max file size. And even if it removed just the ones that are bigger then max file size, IMHO we should not commit such patch because of the reason I mentioned above.

I really appreciate the work on this and I think we are close, but my opinion is that we should not add something that breaks current functionality.

mtodor’s picture

mtodor commented
Assigned: Unassigned » mtodor
mtodor’s picture

mtodor commented
Assigned: mtodor » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2827120_interdiff_22_17.txt1.34 KB
new 2827120_22.patch2.6 KB

Here is solution for proposal from #18.

Basically - it will submit dropzone form, only when valid files are in dropzone.

  • Primsi committed cbc02bb on 8.x-1.x authored by mtodor 
    Issue #2827120 by chr.fritsch, mtodor, Primsi: DropzneJS is cleared on...
primsi’s picture

primsi commented
Status: Needs review » Fixed

Tested and committed. Fixed two small code style nitpicks along the way. Thanks for work!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.