Open crystalenka opened 2 years ago
thumbnails would definitely be my preferred solution
Ping @dgrammatiko 😄
I had some ideas: https://github.com/joomla-projects/media-manager-improvement/issues/510
Related issue: #28392 .
Thumbnails and some kind of "vue smart scroll" (render only visible data) would really improve UX for large amount of files in view.
@richard67 actually #28392 is for J3.x Media Manager and although there are similar approaches to both PHP implementations, the v4 should have a way better performance but unless the getFiles is not streaming chunks of JSON the problem is not solved.
The front end fixes are extremely easy, add and Intersection observer and append the src
when in view
I still have the impression that the thumbs generation code exists in the local adapter (or it existed in the Dropbox) but even if doesn't exist it's dead easy to add it and fire it on image save
unless the getFiles is not streaming chunks of JSON the problem is not solved
It not really JSON problem, but more about client-side rendering of huge list.
Joomla 3 Media have an issue that PHP read whole /images recursively every time, even when it render only 1 specific folder with 2 files. I did not looked, but I hope Joomla 4 does not have it :smile:
It not really JSON problem, but more about client-side rendering of huge list.
It's a 2 fold problem:
I did not looked, but I hope Joomla 4 does not have it
No J4 only get's the file info for each file, which theoretically is orders of magnitude faster than reading the whole file
I had some ideas: https://github.com/joomla-projects/media-manager-improvement/issues/510
A lot of this is above my head but I think it would go far in fixing the issues I mentioned. It's worth a try? Did you get so far as a PR at any point? I'd be happy to test, this is happening on one particular site and managing it is downright painful right now.
@crystalenka @dgrammatiko or maybe a simple pagination, what do you think? (with thumbnails of course) Let say 100 files per page, That should solve huge JSON and rendering.
@crystalenka @dgrammatiko or maybe a simple pagination, what do you think? (with thumbnails of course)
Let say 100 files per page,
That should solve huge JSON and rendering.
Why not both? 🤷🏻♀️
I have 100 files per folder right now and it's taking literal minutes to load. I think if we have pagination, that the # should be configurable because it really depends on the site.
Did you get so far as a PR at any point?
I think we were discussing either the streaming solution or the pagination for phase 2 of MM. Then the team lost any motivation so nothing ever happen.
Let's cut this to 3 different issues so it's easier to get results:
The first 2 should be low hanging fruits the third might need some discussion. No promises but I will try to patch the first 2 (but not this week, quite busy atm)
I'm just happy this is getting some attention and discussion. Thank you!! I'll test any patches when they are ready.
yes i have a site with many many pdf => same problem, its realy realy long (pdf doesn't have preview not realy a problem of image ??)
hello any news about pagination ? it realy an importement issue to resolve ... sorry i can't code it but i can test it
This is how I would envision pagination in the media manager.
@obuisard for starters clone and merge https://github.com/joomla/joomla-cms/pull/36552. I think it's already in a mergeable state.
About the pagination: I would prefer the streaming approach https://github.com/joomla-projects/media-manager-improvement/issues/510 as it is more inline with the SPA of the media manager...
Thank you @dgrammatiko. Looking into developers who could help with this. You have more expertise in that aera than I do, and it would be great if you could lead on this :-) I will clone #36552 for 4.3 and test it out later today.
@obuisard let me give you a rough idea of what I'm talking about with this stream
approach:
Instead of the standard json the media manager (for the getFiles fn, but can be used for more methods) will use the ndjson: http://ndjson.org. In short instead of the current json
will use this
Why? Because the ndjson can be streamed and collected and recreated in the client side extremely easily.
Ok but how? Well, server side all the adapters need to expose if they support streaming or they will default to the current behaviour. If they support (this is a dev option but maybe people might want to expose it in the filesystem plugin options) then instead of using the current code https://github.com/joomla/joomla-cms/blob/614a463b99811789515a4c7c93c2c92c80e10149/plugins/filesystem/local/src/Adapter/LocalAdapter.php#L153-L164 will use generators like https://gist.github.com/EvilWolf/59aa13c51ce1c557c67ad8e3bd4d725d#file-gendirectoryrecursive-php-L4-L27. So essentially the controller should pass an option (the boolean for the adapter stream support) https://github.com/joomla/joomla-cms/blob/614a463b99811789515a4c7c93c2c92c80e10149/administrator/components/com_media/src/Controller/ApiController.php#L132 to the model. (obviously there's more nuance here as the current controller waits for the whole data before forming the response, but hopefully you get the idea).
What about the client side changes? Here the changes should be less. The providers (the adapters) should expose if they support streaming as I said above, this would be taken into account in the fetch of the files https://github.com/joomla/joomla-cms/blob/614a463b99811789515a4c7c93c2c92c80e10149/administrator/components/com_media/resources/scripts/store/actions.es6.js#L28-L46 and instead awaiting for the complete response here https://github.com/joomla/joomla-cms/blob/614a463b99811789515a4c7c93c2c92c80e10149/administrator/components/com_media/resources/scripts/app/Api.es6.js#L71 the store will be updated for each chunk (this article is a good reading on how this can be done https://www.bitovi.com/blog/faster-page-loads-how-to-use-ndjson-to-stream-api-responses although all browsers now support https://caniuse.com/mdn-api_transformstream which makes things even easier).
I hope this somehow clears up what I proposed 4 years ago and if the project decides to go that way then is helpful for the implementors!
The stream approach doesnt change the functionality of the media manager - ux The stream approach doesnt change the performance of the media manager - previews/thumbnails
Its just ignoring the ux and moving the performance issues to a different codebase that once again no one has any experience in so it is impossible to maintain
The stream approach doesnt change the functionality of the media manager - ux
Of course it doesn't as it shouldn't, this is a low level optimisation of the Request/Response
The stream approach doesnt change the performance of the media manager - previews/thumbnails
Actually it DOES and will work even better when someone merges the https://github.com/joomla/joomla-cms/pull/36552 which covers the thumbnails part. @brianteeman before assuming that something won't work it's better to understand the proposal, so read https://www.bitovi.com/blog/faster-page-loads-how-to-use-ndjson-to-stream-api-responses or just check this gif:
moving the performance issues to a different codebase that once again no one has any experience in so it is impossible to maintain
You're speculating that there's something magic but actually there's nothing that PHP developers are not aware already (generators). The other parts ndJSON and the patches are nothing special or even weird. There's nothing weird in this proposal, it's just Joomla never used the fundamental feature of the web: the streams...
Thank you, Dimitris @dgrammatiko, for those explanations and the references throughout. Lots to read :-)
I would suggest to stick to regular pagination, let say ~50-100 files per page.
@dgrammatiko what you have sugested not very perfomance friendly for media manager, and it would introduce unneded complication on server and client side. Good old pagination will solve this issue much better, and will be more easy to support.
what you have suggested not very performance friendly for media manager
What's not very performance friendly
? The client side will render things progressively meaning that the response will be instant and as a user scrolls more items are revealed (of course if you expect that scrolling to the end of the list immediately that won't happen with either the streaming or the paginated view)
and it would introduce unneded complication on server and client side
I think you're assuming that the paginated views are easier to be implemented but the reality is far from that. The client side (the SPA) has its own router that needs to be pathed to support paginated routes. Also the store probably needs some patching as well. On the other hand the only patch for the SPA for the streaming approach is the fetch method and the rest are just PHP changes that most people are quite familiar with.
Anyways, I honestly don't care which solution people will decide to implement, not my problem anyways
What's not very performance friendly?
Rendering: You suggest to dump a whole file list, reqursively. It does not mater how it will be send to browser as stream or all at once, it will be a horror. Usablity: It the same as "endless scroll", which gives you Zero control over content. Example: You have ~2000 files, and want to select file that let say №1753 in that list, and then №1767. With pagination you quickly go trough pages and pick your first file, then open manager again, click last selected page and chose your next file. With the stream approach you will hate Media manager very quickly :neckbeard:
You suggest to dump a whole file list, reqursively. It does not mater how it will be send to browser as stream or all at once, it will be a horror.
Nope that's not what I'm proposing. Even the current code in the media manager is not collecting the dirs/files recursively so I really don't know how you came up that I'm suggesting that there will be one request that will dump the hole tree. It's not like that right now and definitely is not what I had in my mind...
Usablity: It the same as "endless scroll", which gives you Zero control over content.
There's a search field...
Anyways, I just wanted to clarify that the first assumption is totally wrong...
is not collecting the dirs/files recursively so I really don't know how you came up that I'm suggesting that there will be one request that will dump the hole tree
Yeah, sorry about recursion, I misread something :) But my point still valid, it does not speed up much, you still need get all files in folder before submit them, and then render all (even if it in stream), UI still may be unresposive with huge amount of list(files) in frame, after stream will be fully rendered.
And sending response while gathering a files not going to happen, for many technicla reasons: there a plugin event that need to be trigered for files list, then there how Joomla render component and more.
Is your feature request related to a problem? Please describe.
When using the native Joomla 4 media manager to select an image, there are several usability issues as an end user:
Describe the solution you'd like
There are several possible solutions that would help mitigate the slow loading - any or all of them would help:
I don't know how to fix not being able to navigate folders until the first one is done loading.
Additional context
Before opening this issue I did some looking around at past issues. This has been a hot topic in the past without any resolution for over a year, it would be nice to solve it now. :)
I noticed that the media manager is not loading inline images (with
<img>
tags), but instead it's setting the images as background images of divs. This seems inefficient but I'm not sure of the direct performance impact. This would definitely be a roadblock to adding native lazy loading though.I would open a PR to try to solve some of these issues but it seems the media manager is written in Vue.js, making it completely beyond my skill set.