Remove MultiUpload Dialog

[bezumkin]

What does it do?

• Updated mailru/FileAPI to version 2.0.25
• Disabled use of Flash for upload and removed its files.
• Removed MultiUpload Dialog because it requires too many actions from user.
• Upload button now opens browser upload dialog.
• Added drop zones in Packages and Media Browser panels.
• Added ability to delete multiple files in Media Browser.

Why is it needed?

To improve user experience when working with files.

Related issue(s)/PR(s)

N/A

Demo

Click to images to see this changes in action

[joeke]

@bezumkin Awesome! Does it also support selecting multiple files and moving them by drag/drop to another folder?

[bezumkin]

@joeke No.

It is too complicate to do it with ExtJS for me.

[matdave]

Awesome job @bezumkin

[Oetzie]

@bezumkin nice PR! But can you make it a little more user friendly with a message like 'Or drag n drop files here' like something on the screenshot I did send to you by WhatsApp? A normal website editor (a noob) won't know you can upload (multiple) files with drag n drop on that spot?

[bezumkin]

As I see, you have special drop zone with message. But I just make the whole panel to be drop zone, and I really don`t see where this message could be.

If somebody will show me example on image - I will try to do it.

And, btw, current MultiMedia Dialog also supports dnd and there are also no messages about it =)

[Oetzie]

@bezumkin I will try to make an example.

And, btw, current MultiMedia Dialog also supports dnd and there are also no messages about it

thats why MODX 3 is there to make the UI better right? ;-)

[bezumkin]

Added DnD into Browser Window and RTE iframe.

Click to the previews to see GIFs:

[Oetzie]

@bezumkin how does it work with other RTE editors? It is the same browser so it will work for all RTE editors I guess?

[bezumkin]

@Oetzie It should, because I changed the common browser view, that used by other components!

But I hope somebody with test this PR with his favourite RTE.

[Oetzie]

@bezumkin Could you test it with https://github.com/Oetzie/TinyMCE?

[bezumkin]

@Oetzie It works!

[Oetzie]

@bezumkin Awesome!

[bezumkin]

@Oetzie Added background image

This don't require changing of ExtJS panels and lexicons but, I hope, pretty understandable. Image was made by @sipkogroefsema

[arjen-t]

I tested functionality locally all seems working well! One side effect detected.

This also exists in repo 2.*.

https://github.com/bezumkin/revolution/blob/48befcaaa295d1e246f3cb8735c9ce73008ae4f8/manager/assets/modext/util/multiuploaddialog.js

This component is used in the tree.directory component but without the defined setting of 'permitted_extensions'.
There can be a rare case of a strange user experience.

Side effect:
User experience isn't in sync with the real outcome of the functionality.
Developers are wondering whats going on?

User behaviour:
If a user is switching mediasources back and forward in the Media Browser.
The user can only upload files with an extensions which are accepted via the system setting 'upload_files'.

Real outcome:
In rare case the default fallback for 'permitted_extensions' via the system setting 'upload_files' (in the upload component) can block a file extension
which actually is accepted by the backend via the 'allowedFileTypes' property of the selected media source.
This behaviour appears because the 'allowedFileTypes' property is selected above the 'upload_media/files/images/flash' settings. This is also the case in repo 2.*. (See the checkFileType method in the mediasource classes.)
Right now the user can't upload the file because extjs is preventing so but technically the user is allowed to do so.

Solution:
Sync the client behaviour.

Possible solution;
Add in the response (from core/processors/source/getlist) via the MediaSource combobox by each media source the property 'allowedFileTypes' based on the logic of the method checkFileType?

And the tree.directory component update's the 'permitted_extensions' of MultiUploadDialog.Upload when the resource combobox is changed by the user.

[bezumkin]

As I see, allowedFileTypes is not for uploading files, but for displaying them

permitted_extensions controlled inside ExtJS to limit selecting files in various situations. For example, when we upload packages in Installer - it limited to only zip

[arjen-t]

@bezumkin thnx for your feedback.
I checked more in detail and I misinterpreted two things.
First one which you pointed out about the 'allowedFileTypes' property. You are correct about this one, mine bad.
Second about the 'checkFileType' method in the mediasource class.
I interpreted/misunderstood the code the wrong way,
so technically in this method there is a merge happening based on the 'upload_media/files/images/flash' system settings which result in a array of allowed extensions.

Correct me if I'm wrong, but still some weird behaviour can appear for the user.
Right now the user can upload any sort of file with the restriction based on the 'permitted_extensions' property of the upload extjs component.

For example; Me as a webmaster define a media source which only accepts png/jpg by setting the 'allowedFileTypes'.
A user is going to upload a pdf file. The 'permitted_extensions' property of the upload extjs component is allowing him to do so. (system settings).
At the moment of upload all goes fine no error appear for the user. But after the folder refresh his pdf file doesn't appear because it got filtered out by the 'allowedFileTypes' property of the media source.
I can imagine a user thinks where is mine file let's try again maybe something went wrong and at the end he gives up and will contact the webmaster/web company.
Second of all the pdf file got uploaded and is hanging around on the server without any usefull usage. (read; can't be deleted by the user because he can't see it)

I don't know if this is desired behaviour and second of all if this is or was a conscious design desicion.
I'm pointing this out because I read the stories and threads in the community to rewrite this media source part to make it more user friendly. And I think this behaviour can be confusing for users.

[bezumkin]

but still some weird behaviour can appear for the user.

Yes, you are right. I will think how to fix it.

[arjen-t]

My 2 cents

I think the best way to controll the mediasources on a proper level is to define properties in the media source it self which effect the upload.
I faced already a couple of times with a project that we actually want to have multiple mediasources for different purposes, eg; different usergroups or allowing different type of files per mediasource etc.
We noticed this is not possible to achieve with the standard solution. (of course besides the usergroups access).

I think if we start to support per mediasource which extensions are allowed to upload.
This gives the developers most of there use cases out of the box.

Per mediasource decide what is allowed to upload and which usergroups can access the mediasource.

And treat the settings as a cascade.

mediasource extension property defined?

yes -> mediasource property leading in upload
no -> system setting leading in upload (standard logic of merging the properties as it is)

I think this gives the possiblity to keep it reverse compatible and letting developers start to use this meganisme if desired.

Overall thumbs up for the rewrite the upload experience looks way better than before, thnx for that!

[bezumkin]

@Atid I agree with you, but I think that it is question to Media Sources, not to Upload component.

I suggest you to create a new issue about it after merging both of them to 3.x branch, because it is definitely needs to have a discussion.

[arjen-t]

@bezumkin I agree with you this topic is touching both merges. When both are merged I will open a issue.

[gpsietzema]

Did a lot of testing and did find some small things, but nothing that should prevent this from being merged. That can be part of alpha/beta testing if you ask me.

[bezumkin]

@gpsietzema Sorry, I rebased this PR to make it easier to merge it.

[Mark-H]

Nice work Vasily!