Make WordPress Core

Opened 12 years ago

Last modified 6 years ago

#23205 new enhancement

New Media Uploader slow for sites with many images

Reported by: salromano's profile salromano Owned by:
Milestone: 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 years ago.
Patch for media-views.js to set default to attached images
23205.diff (943 bytes) - added by adamsilverstein 12 years ago.
23205.2.diff (918 bytes) - added by adamsilverstein 12 years ago.
corrected indentation for improved legibility
23205.3.diff (1.8 KB) - added by creativeinfusion 11 years ago.
Apologies, this replaces the incorrectly generated patch file

Download all attachments as: .zip

Change History (38)

#1 @toscho
12 years ago

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

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

#2 @SergeyBiryukov
12 years ago

  • Milestone changed from Awaiting Review to 3.6

#3 @ryan
12 years 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.

#4 @ryan
12 years ago

  • Severity changed from critical to normal

#5 @creativeinfusion
12 years 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.

#6 @creativeinfusion
12 years ago

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

#7 @salromano
12 years 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?

#8 @ocean90
12 years ago

  • Cc ocean90 added

#9 follow-up: @vickybiswas
12 years 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.

@vickybiswas
12 years ago

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

#10 in reply to: ↑ 9 @adamsilverstein
12 years 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.

thanks for the patch!

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.

Last edited 12 years ago by adamsilverstein (previous) (diff)

#11 @husobj
12 years 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.

#12 @adamsilverstein
12 years ago

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

#13 follow-up: @SergeyBiryukov
12 years 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]).

@adamsilverstein
12 years ago

corrected indentation for improved legibility

#14 in reply to: ↑ 13 @adamsilverstein
12 years 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

#15 @ocean90
12 years ago

  • Version changed from trunk to 3.5

#16 follow-up: @markjaquith
11 years 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.

#17 in reply to: ↑ 16 ; follow-up: @adamsilverstein
11 years 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?

#18 in reply to: ↑ 17 ; follow-up: @nacin
11 years 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.

#19 @creativeinfusion
11 years 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)?

#20 in reply to: ↑ 18 @husobj
11 years 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.

#21 @creativeinfusion
11 years 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.

@creativeinfusion
11 years ago

Apologies, this replaces the incorrectly generated patch file

#22 follow-up: @nacin
11 years 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?

#23 @creativeinfusion
11 years 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.

#24 follow-up: @nacin
11 years 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.

#25 in reply to: ↑ 24 @alexis.nomine
11 years 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.

#26 @ocean90
11 years ago

#25804 was marked as a duplicate.

#27 @leemon
11 years ago

  • Cc tacrapo@… added

#28 @ocean90
10 years ago

#28901 was marked as a duplicate.

#29 @nimmolo
9 years ago

Could this be green-lighted for core already please? 3 years!

Particularly in the case when a user clicks the "Featured Image" button, I can't think of any reason they'd ever expect not to get "uploaded to this post" as the default.

Last edited 9 years ago by nimmolo (previous) (diff)

#30 in reply to: ↑ 22 ; follow-up: @nimmolo
9 years ago

Replying to nacin:

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.

The remembering doesn't work in my use case. Try going through the old posts of a big blog where you've changed themes, and assign a featured image to a batch of 200 old posts. It doesn't remember your "Uploaded to this post" choice between post edit pages, and keeps defaulting to the most recent media uploads.

I think this example is an increasingly relevant case: the maintenance and updating of an existing blog that maybe didn't previously make use of featured images or galleries. Repetitive work that could be easier.

The default behavior is only relevant (and maybe kind of expected) when you're adding posts on the top (most recent post) of a blog, but again in this case I think users really only expect to default to the images they've uploaded to the post.

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?

It's way slower in the case of an old normal blog post with a few images, because Media Uploader is now pulling 40 recent images at medium size instead of, say, the one or two or three relevant images in the post itself. Compounded over the revision of two hundred posts, this is a lot of extra time and bandwidth.

Last edited 9 years ago by nimmolo (previous) (diff)

This ticket was mentioned in Slack in #core-flow by boren. View the logs.


9 years ago

#33 in reply to: ↑ 31 @nimmolo
9 years ago

Last edited 9 years ago by nimmolo (previous) (diff)

#34 @ryan
9 years ago

I opened #33773 - Media, Featured Images: The media modal doesn't remember filter settings in the context of featured images

Note: See TracTickets for help on using tickets.