WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 6 months ago

#23205 new enhancement

New Media Uploader slow for sites with many images

Reported by: salromano Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: dev-feedback has-patch
Focuses: Cc:

Description

The new media uploader added in 3.5 looks very nice. Unfortunately, its functionality is worse for sites with thousands of images. I think this can be combated by allowing us to select the "Upload Files" page as our default after clicking "Add Media," and rather than "All Media Items" be the page it jumps to after the image is uploaded, instead have it jump to "Uploaded to this Post." Is there any way the WordPress team can make this default? Or add the option to make it default? It's made posting on my site 10 times more annoying, especially for those posts with 40-50 images. And I'm sure there are others who feel the same.

Attachments (4)

defaultview.patch (835 bytes) - added by vickybiswas 12 months ago.
Patch for media-views.js to set default to attached images
23205.diff (943 bytes) - added by adamsilverstein 12 months ago.
23205.2.diff (918 bytes) - added by adamsilverstein 11 months ago.
corrected indentation for improved legibility
23205.3.diff (1.8 KB) - added by creativeinfusion 9 months ago.
Apologies, this replaces the incorrectly generated patch file

Download all attachments as: .zip

Change History (31)

comment:1 toscho15 months ago

  • Cc info@… added
  • Keywords needs-patch added

I second that. Uploaded Files should be the default view.

comment:2 SergeyBiryukov15 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:3 ryan15 months ago

The modal currently remembers the last page you had open, but once you upload from "Upload Files" the last page becomes "Media Library/All Media Items". Perhaps it could remember the last source filter that was explicitly clicked rather than the last one you are redirected to.

Also, you should be able to drag files onto the media library even while images are being fetched.

comment:4 ryan15 months ago

  • Severity changed from critical to normal

comment:5 creativeinfusion14 months ago

  • Cc wptrac@… added

+1 to allow a default of "Uploaded to this post".

I'd quite like to be able to disable the All Media Items view as well.

Although the workflow for creating shortcode galleries in blog posts is obviously much better in the new media manager and allowing reuse of images loaded elsewhere is a fine idea, it has unfortunately severely compromised the user workflow when the site is not so blog-centric. For example we often make custom themes that automate galleries (e.g. create a custom post type for a product and use any images uploaded to each product to automatically show a gallery on the single-{post_type}.php and a featured images on an archive-{post_type}.php). In this use case the All media items view is confusing to the user, makes the workflow longer and as the OP has noted rapidly slows down sites with lots of images.

I'd love to see some filters that allow us to select different behaviour, preferably allowing different choices for different post types.

comment:6 creativeinfusion14 months ago

Related performance issue (especially on sites with a lot of large images) #23292

comment:7 salromano14 months ago

Any chance we could get this in 3.5.2 rather than 3.6? Certainly changing the default view of something shouldn't be too big a task?

comment:8 ocean9013 months ago

  • Cc ocean90 added

comment:9 follow-up: vickybiswas12 months ago

  • Cc vickybiswas@… added
  • Keywords has-patch added; needs-patch removed

Here is the patch
The state machine initializes by default in ALL state.
A set of library default values helped there.
Also the featured image part I did separately as we might at a later time want to have a different default there.

@salromano I know now why such a "simple" task was not done. It took me time to understand however much I know JS I cant do it without understanding backbone js. Also backbone js was not a piece of cake to understand.
However that I understand it now the solution looks simple.

vickybiswas12 months ago

Patch for media-views.js to set default to attached images

adamsilverstein12 months ago

comment:10 in reply to: ↑ 9 adamsilverstein12 months ago

Replying to vickybiswas:

Here is the patch
The state machine initializes by default in ALL state.
A set of library default values helped there.
Also the featured image part I did separately as we might at a later time want to have a different default there.

@salromano I know now why such a "simple" task was not done. It took me time to understand however much I know JS I cant do it without understanding backbone js. Also backbone js was not a piece of cake to understand.
However that I understand it now the solution looks simple.

i tested your patch and it works as expected; i get the 'attached to this post' selection as the default; not certain this is the best approach, but it works for me.

23205.diff is a slight cleanup of your patch -i regenerated from the root - its best to create patches from there so they are easier to apply; also cleaned up whitespace a tiny bit.

Version 0, edited 12 months ago by adamsilverstein (next)

comment:11 husobj12 months ago

+1 for making this the default behaviour again or at least providing a filter so that it can be made the default behaviour if preferred.

I've certainly found this a performance issue on sites with large images and also the workflow is not intuitive on some sites that auto-create and display galleries based on image attached to a post.

comment:12 adamsilverstein11 months ago

  • Cc adamsilverstein@… added
  • Keywords dev-feedback added
  • Version set to trunk

comment:13 follow-up: SergeyBiryukov11 months ago

23205.diff would be a bit more readable if there's only one level of indentation for both arrays (like in [23598] and [23668]).

adamsilverstein11 months ago

corrected indentation for improved legibility

comment:14 in reply to: ↑ 13 adamsilverstein11 months ago

Replying to SergeyBiryukov:

23205.diff would be a bit more readable if there's only one level of indentation for both arrays (like in [23598] and [23668]).

thanks for catching that; corrected in 23205.2.diff

comment:15 ocean9011 months ago

  • Version changed from trunk to 3.5

comment:16 follow-up: markjaquith10 months ago

  • Keywords needs-patch added; has-patch removed

Would rather that only explicit choices be remembered, rather than change the default for everyone, as the patches do.

comment:17 in reply to: ↑ 16 ; follow-up: adamsilverstein10 months ago

Replying to markjaquith:

Would rather that only explicit choices be remembered, rather than change the default for everyone, as the patches do.

all this patch seems to do is change the default: before any user chooses an option the default is currently all images, this changes that to uploaded images; right? or does it change the behavior in some other way? will this actually overwrite the settings a user last used?

the issue of changing the option changing it for all users seems like a separate issue?

comment:18 in reply to: ↑ 17 ; follow-up: nacin9 months ago

Replying to adamsilverstein:

Replying to markjaquith:

Would rather that only explicit choices be remembered, rather than change the default for everyone, as the patches do.

all this patch seems to do is change the default: before any user chooses an option the default is currently all images, this changes that to uploaded images; right? or does it change the behavior in some other way? will this actually overwrite the settings a user last used?

the issue of changing the option changing it for all users seems like a separate issue?

No, that's this issue. That's a behavior change. All images should be the default. It's the most obvious option. Changing the default is not ideal here.

Rather: There *is* some code to remember where a user last was, and use that for the future. But, it seems like that is not happening here. That is the problem, not the global default.

comment:19 creativeinfusion9 months ago

Would having a filter to change the default be acceptable as per husobj suggestion?

We auto create galleries on most sites to streamline the workflow and have more control over presentation, and as husboj has noted the new approach is making that problematic, especially on image heavy sites.

The previous uploader didn't force a page load of all the images as the Media Library link was only followed if you actually wanted to see all images - there was a gallery link that just showed what was attached to that post. So I suppose the behaviour change has really happened as a result of routing the 'uploaded to this page/post' images through the Media Library, rather than having it available separately.

So alternatively, how about restoring a separate link for images uploaded to this page - it's not an obvious place to have it under Media Library anyway (if I recall correctly it was a later addition to the new uploader)?

comment:20 in reply to: ↑ 18 husobj9 months ago

Replying to nacin:

No, that's this issue. That's a behavior change. All images should be the default. It's the most obvious option.

It's true that it is the most obvious option and I understand that is why it has been made the default view, but it is not always the most user friendly experience for all sites/users.

I've installed many WordPress sites for a variety of clients (mainly CMS sites not blogs) and from my experience can think of very few use-cases where a user creates/edits a page and wants to "add media" and select an existing image from the media library. In my experience they are generally looking to add new images or manage existing images that were uploaded via that page.

That's particularly true in the case of pages with auto-generated galleries as mentioned in my comment above.

I have no issue with this being the default view as long as there is a filter that can be used to override this if desired.

There is some JavaScript I found somewhere which can be used to switch to the "uploaded to this page" view by default, but it doesn't trigger until all the images have loaded which can take a long time so is not really a valid solution.

I would also be happy with a JavaScript solution if it avoided having to wait for all images to load before switching.

comment:21 creativeinfusion9 months ago

  • Keywords has-patch added; needs-patch removed

Hope I've done that patch right as I've never done it before.

The basic idea is to use a filter 'media-default-view' with a return value of 'post' to make the default be 'Uploaded to this post' otherwise behaviour is unchanged.

This is a more user friendly default for some CMS style sites as discussed earlier.

creativeinfusion9 months ago

Apologies, this replaces the incorrectly generated patch file

comment:22 nacin9 months ago

In media-views.js, check out the saveContentMode function and the libraryContent user setting. This stores the user's last visited screen in certain situations. Does that work for this situation? Does it remember your choice of the uploader after you go to it? If not, then let's fix that for the situation described. If it does, then I'm not sure what else to do here.

But more so, secondly: I would like to know how it is *slower*. It only pulls down a few dozen images, and only pulls down more as you scroll. Is it the JS, the SQL query itself, etc?

comment:23 creativeinfusion9 months ago

The libraryContent user setting remembers the mode, i.e. 'upload' or 'browse', that the user is on when they exit the "Insert Media" thickbox. This is whether the user was last on the "Upload Files" or "Media Library" tab respectively (I haven't tested whether it would remember a theme/plugin defined tab made using the media_upload_tabs filter but that's not especially relevant to this). As after uploading a file the view automatically switches to Media Library, All Media Items when the user exits the from the "Insert Media" thickbox after an upload the remembered setting is 'browse' as indicated by ryan.

The choice of the subset on the Media Library screen dropdown, i.e. All Media Items, Uploaded to this post/page, Image, Audio, Video or Flash is not remembered. I'm not convinced it'd be logical to store it as a user setting as, for example, looking at Audio items on an Add Media screen for a custom post type does not necessarily imply anything about whether they want that on a Media Upload to a page.

The OP wanted to be able to default to change the default Insert media screen to be the Uploaded screen but it looks to me that if controlling this is a real issue then in principle the get_user_option_user_settings filter should be able to handle it.

The OP and others (e.g. me!) also wanted to be able to control the default Media Library selection subset (for me this is a client workflow issue when editing on CMS style sites rather than a speed one). There's no existing mechanism to do that, hence the filter in the latest patch to restore the simpler workflow we used for such sites pre 3.5. What's the issue with restoring this, the fix is simple and has no impact on anything else - I don't see a down side?

Secondly regarding speed. There's a short pause waiting for the ajax query to fetch the first 40 images which is more of an issue if a site uses quite large dimensions for the 'medium' image size as that's the one the media view uses. This would be dealt with for me if we could choose to use the 'thumbnail' size instead, but there's no option to do that. There's a separate performance issue in ticket:23292 about uploading large images and the preview using the large image. Maybe others can comment on what's causing their speed issue.

comment:24 follow-up: nacin9 months ago

  • Milestone changed from 3.6 to Future Release

I think this is mostly a duplicate of #23292, unless this refocuses toward better remembering of your last screen.

As stated in #23292:

  • By default medium size images are loaded.
  • If images don't have medium sizes, or if the medium size is configured oddly (such as 0 x 0, to kill the size), then full will get used. The default dimensions for thumbnails are simply too small for the new UI.
  • I think there are some good ideas here for a future release. It is not a pressing issue at this point.

comment:25 in reply to: ↑ 24 alexis.nomine9 months ago

Replying to nacin:

I think this is mostly a duplicate of #23292, unless this refocuses toward better remembering of your last screen.

Sorry to jump in the discussion like this.
I think the last patch from 'creativeinfusion' allowing to restore the pre 3.5 media behavior from a filter is really useful for CMS installations of WordPress. Currently, people are trying to find hacky workarounds to select "uploaded to this post", and some users are forced to have one annoying step of selecting it manually each time, which can be quite annoying in some cases.
This is not so much about performance but more about usability and I think a lot of users would be happy to have at least the possibility to change the behavior via a filter.
I'd be sad to see it not going in 3.6 as the patch is ready to go, and doesn't break anything.

comment:26 ocean906 months ago

#25804 was marked as a duplicate.

comment:27 leemon6 months ago

  • Cc tacrapo@… added
Note: See TracTickets for help on using tickets.