search

FriendsOfFlarum / upload

The file upload extension with insane intelligence for your Flarum forum.
https://discuss.flarum.org/d/4154
MIT License
176 stars 95 forks source link

Implemented user media manager #262

Closed jaspervriends closed 3 years ago

jaspervriends commented 3 years ago

I've implemented a user media upload manager with the idea of stopping multi-uploads from the same file. The files were already attached to the user, so I did not change anything of that.

Changelog:

Modal features: The media manager is expandable minded. Other extensions could open the FileManagerModal modal component and attach custom params to customize their needs.

The file manager is responsive, has drag & drop support and automatically selects the uploaded files. After clicking the 'Select files' button, the files will be added with their bbcode to the composer.

This PR Let's focus on the code and improvements/removals:

Updated composer buttons

image

Automatically select uploaded files

image

Example: Restricted to file type image

image

Night mode

image

Mobile view

image

imorland commented 3 years ago

This really is great work @jaspervriends thank you for your time in creating this feature! Might take a couple of days to go over this, please bear with us :)

clarkwinkelmann commented 3 years ago

This looks very nice! I probably also won't be able to review immediately, but I will try to get to it!

How are the image preview managed? Is it the full image? If so, does it load all of the images immediately or only as you scroll down? If a forum allows very large uploads, or someone just has many images, will it not bring the website down by loading all of them?

jaspervriends commented 3 years ago

Awesome, take your time :)

This looks very nice! I probably also won't be able to review immediately, but I will try to get to it!

How are the image preview managed? Is it the full image? If so, does it load all of the images immediately or only as you scroll down? If a forum allows very large uploads, or someone just has many images, will it not bring the website down by loading all of them?

The files are loaded in batches as usual (20 files each), so it will depend on the overal image size. Currently the images are just using the path of the image. As there are no thumbnails generated by this extention :(

So if someone has any suggestions, let me know. Maybe generate it on-the-fly on a specific endpoint? fof/uploads/{file-id}/preview? If so, would we cache that on the server, or just in the browser?

luceos commented 3 years ago

I'd love to hear other peoples' opinion about the following. Jasper and I had a discussion about the upload button, previous commits had it replaced by the media manager. Right now there's (yet) another button. Screen space is scarce, especially in the editor.

My suggestion is to allow an admin setting to choose between (upload button, media manager button or both). And then render accordingly. The media manager adds another click to upload.

davwheat commented 3 years ago

I'm a fan of that. Maybe it's time to also replace the "Upload" text with a tooltip to match the other buttons in the editor toolbar?

jaspervriends commented 3 years ago

@luceos I think an admin setting would be fine. What would be the default setting then? Both buttons visible? And optionally hide one of the buttons?

Maybe it's time to also replace the "Upload" text with a tooltip to match the other buttons in the editor toolbar?

Yup, I already gave the upload button a tooltip :)

jaspervriends commented 3 years ago

@luceos @davwheat As requested, I've made some changes and worked on the feedback provided.

I've added an option called Composer buttons to the admin dashboard: image

Which results into:

If there is anything else, let me know!

davwheat commented 3 years ago

Not sure on others' opinions, but it might be worth considering display: grid again. Flarum doesn't use it elsewhere, but some other styles core and this ext use are incompatible with IE11, so using it wouldn't be a major issue, but it'd just make styling far easier.

Up to you at the end of the day -- both work fine!

jaspervriends commented 3 years ago

@imorland I've updated the PR again and added an uploads overview to the user profile page (I agree it is a good addition too). Also, I added a fof-upload.viewUserUploads to view uploads from other users for moderation purpose. By default the permission is set to moderators in the migration file.

Also, I already added the fof-upload.deleteUserUploads to the same migration file for future usage. There is no delete functionality yet, also, I am wondering what would be a good approach for this as there are multiple ways to upload files to different adapters.

When a user sees their own profile, the button label is My uploads, when viewing another profile (and having the rights) they'll see User uploads.

When you click a file in the user profile page, it will open the file URL.

image

imorland commented 3 years ago

Superb @jaspervriends - I'll take a closer look after work tonight!

In regards to deletion, I agree it's a tricky subject. My feeling is that instead of actually deleting the file from disk/bucket/whatever, which in turn could likely impact posts that have it embedded, we should instead 'delete from media manager'.

Perhaps this could be done by adding a column to the fof-upload-files table, null by default, to act as a flag as to if the file should be included in the media library?

imorland commented 3 years ago

Any thoughts on the above @jaspervriends ?

Ideally I'd like to get this merged before we start working on the beta 16 updates for this extension

imorland commented 3 years ago

I'll merge this now, @davwheat and I will add the 'delete from media manager' feature as described https://github.com/FriendsOfFlarum/upload/pull/262#issuecomment-777415577