fix(pat contentbrowser): Only close the upload panel when upload was successful

[frapell]

fixes gh-1491

[petschki]

If I understand correctly, you're loading the complete level listing and show the preview of the uploaded item.
I have two concerns about that:

1. what if I upload multiple items? the success event gets triggered after each upload... (see pat-upload event when all uploads are completed #1445)
2. what if my level has 100 pages?

I also came accross the problem, that the uploaded items aren't shown, when the level is batched. our quickfix in that project was a lifecycleevent which resorts newly added items to the top of the folder listing but that's not OOTB Plone behavior.

my idea for a solution would be:

1. save all upload states with error and success callback
2. when queuecomplete check for the states -> if error do not close upload panel
3. if the level is batched, load only the last number of uploaded items and append it to the bottom (maybe show a diving div with "..." between the current level list and the appended items) and scroll to the bottom.

what do you think?

[frapell]

@petschki Right, I didn't consider the multiple files case, since I was specifically working in an issue when uploading from the insert image dialog in TinyMCE...

My first attempt at solving the issue was indeed adding an error flag that would be set to true in an error callback, and check that in queuecomplete to decide if I could close it or not, but this prevented errors in the upload to be shown in the UI...
Let me try to explain, in our case, we do some checks in the uploaded files, and raise exceptions under certain conditions (File size, naming, etc...). When that happens, the exception message gets printed right below the filename in the upload widget. When I defined the error callback when initializing the pat-upload, something was being overridden not sure where, that prevented the error message being displayed.

In another product, we are doing this for automatically reloading the page if all files were uploaded

    $dropzone.on('success', function() {
      remaining = this.files.length - 1;
    });

    $dropzone.on('queuecomplete', function() {
      if (remaining == 0){
          location.reload();
      }
    });

maybe I could try this approach better?

Now, on automatically selecting the uploaded file... Uploading several files at once raises the question on which should be selected... the last one? should one be selected when the upload is over? Maybe automatically select it if the uploaded file was only one?

On the batching... One solution I tried first, was instead of loading the whole folder until the end, to use the search instead. Calling searchItems(val) with the file name, and then selecting that... I didn't like a few things of that approach:

1. You loose the context... you were like 4 levels deep in a folder, uploaded a file, and all of the sudden you have only the uploaded file visible...
2. What if there are some events fired when the file is uploaded that change the name or whatever, that would make the search not find it...

Talking strictly on TinyMCE "Insert Image" dialog, before changing to contentbrowser, when there was an "Upload" tab, the behavior was that once the file finished uploading, it will switch back to the "Internal" tab with the just uploaded image selected. I don't remember if it allowed you to upload several images at once and what would it do in that case...

Not sure where to go from here :)

[petschki]

@MrTango @yurj @1letter @thet they might have opinions on that ;)

We also had some customer feedback on the upload process, since in pat-relateditems the uploaded items were inserted immediately after uploading (single or multiple). with pat-contentbrowser you had to

1. browse to the location (optional)
2. upload the items
3. select them
4. insert them

because of that, I implemented the auto selection of all uploaded items to get rid of one click at least. But to make my customers happy, the uploaded items should be inserted immediately after uploading. Maybe this could be a new option "add uploaded item(s) immediately to the selected items" (ideas for option name welcome) or something like that to configure how this behavior should be. If this is enabled (maybe default, because of the old relateditems behavior) then the contentbrowser closes immediately after (successful) upload and shows the items in the selected field.

when it comes to tiny image dialog, I'd go with enabling the immediate insert option. btw. the same is for internal links, where you can upload also files and link them immediately after uploading them ...

[petschki]

I will spend some time today here, because the current upload workflow especially in the tiny link/image modals is bothers my customers the most...

improvements

1. "Select" Button needs to be called "Select or Upload" if upload parameter is enabled.
2. The uploaded image/file should immediately be inserted to the selected items and the contentbrowser should be closed ... maybe we can configure that to a parameter immediately-add-uploads which defaults to true in case someone needs a custom behavior on that ... also the "Upload" Button should be renamed to "Upload and Insert" if this immetiate switch is true.

other things

1. Validation errors during/after upload should definitively keep the upload dialog open with the information next to the file.
2. Displaying the files after uploading in the contentbrowser is tricky. Inserting the uploads immediately will solve this partly, but what if you upload 5 items and one has a validation error? The successful uploaded items are inserted, and the failed is displayed in the upload dialog column? At least that's what I would expect ...

Despite of that, we really need a quick fix for the tiny modals, because people keep asking for improvement...

@erral you may also have ideas on this topic...

[erral]

It is like you are reading my mind...

On Friday I had a chat with a client that had this errors:

• Error on upload was not reported
• When uploading a single fiel,he was expecting to be automatically selected to be linked

😅

[petschki]

FYI I've made some improvements here #1507 ...

[petschki]

#1507 got merged ... I'll close this.