Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 21 months ago

#53765 closed defect (bug) (fixed)

Media Library shows only the selected image

Reported by: benitolopez's profile benitolopez Owned by: antpb's profile antpb
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.8
Component: Media Keywords: has-patch commit
Focuses: javascript Cc:

Description

I don't know if this is a bug or if it is by design. But since WordPress 5.8, when you open the media libray that has a selection, only the selected image(s) is shown and other medias are not there. To show again the other images included in the library you need to filter the medias.

Seems that it happens only when using the classic editor. To reproduce the issue, you can set a featured image to a product of WooCommerce (for example). Or to any post type that is using the classic editor.

Previous versions of WordPress don't have this problem.

Attachments (8)

53765-reproduced-issue.gif (4.1 MB) - added by hellofromTonya 3 years ago.
Testing: reproduced when upgrading from 5.7.2 to 5.8.0
53765-reproduced-issue2.gif (6.8 MB) - added by hellofromTonya 3 years ago.
Testing: additional reproduced testing
53765.diff (1.3 KB) - added by joedolson 3 years ago.
Converts notes by szaqal21 into patch.
53765.2.diff (909 bytes) - added by joedolson 3 years ago.
New patch
screen-capture.webm (1.3 MB) - added by szaqal21 3 years ago.
Test scenario
screen-capture (1).webm (685.8 KB) - added by szaqal21 3 years ago.
posts_per_page = 40
no_load_more.png (90.5 KB) - added by szaqal21 3 years ago.
Noticed something else, lowering posts_per_page to 40 caused Load more button dissapear.
screen-capture.2.webm (2.7 MB) - added by szaqal21 3 years ago.
Bug in Featured Image dialog.

Change History (82)

@hellofromTonya
3 years ago

Testing: reproduced when upgrading from 5.7.2 to 5.8.0

#1 @hellofromTonya
3 years ago

Testing Report

Testing Environment:

  • OS: macOS Big Sur 11.4
  • Localhost: Local
  • WordPress: 5.7.2 and the 5.8.0
  • Activated plugins: Classic Editor plugin
  • Theme: Twenty Twenty-One
  • Browser: Chrome Version 91.0.4472.114, Firefox 89.0.2 (64-bit), Safari 14.1.1

Testing steps to reproduce:

  • Install and active the Classic Editor plugin
  • Switch to 5.7.2 (Local testing: use the Core Rollback plugin.)
  • Add multiple images to the Media Library (at least 2)
  • Create a new post
  • Set the Featured Image
  • Save the post
  • Click on the image in the Featured image field to reopen the modal -> notice all of the items in the media library including the selected one are displayed
  • Upgrade to 5.8.0
  • Go back to that post
  • Click on the featured image again to reopen the modal -> notice only the selected image is displayed

Results:

  • Reproduced issue when using the Classic Editor as shown in 53765-reproduced-issue.gif in each browser ❌
  • No errors in browser console ✅
  • error.log is empty ✅
  • Confirmed the issue does not happen with block editor, when Classic Editor plugin is not activated ✅
Last edited 3 years ago by hellofromTonya (previous) (diff)

#2 @hellofromTonya
3 years ago

  • Milestone changed from Awaiting Review to 5.8.1

Hello @benitolopez,

Welcome to WordPress Core Trac! Thank you for reporting this.

I was able to reproduce the issue. It does look like a regression. Moving it into 5.8.1 for further investigation.

@hellofromTonya
3 years ago

Testing: additional reproduced testing

#3 @hellofromTonya
3 years ago

  • Keywords needs-patch added

I was curious what happens when deselecting or removing the featured image. Are users able to get access to the rest of the media library? 53765-reproduced-issue2.gif shows the results of the testing.

Yes, users can regain access but only when following these steps:

  • Remove the featured image
  • Save or update
  • Select to add a feature image to reopen the media library modal

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


3 years ago

#5 @joedolson
3 years ago

Testing report:

Testing Environment:

OS: Windows 10
Localhost: no
WordPress: 5.7.2 and then 5.8.0 nightly 7/27/2021
Activated plugins: Core Rollback, WordPress Reset, Beta Tester
Theme: Twenty Twenty-One
Browser: Chrome 92.0.4515.107 (Official Build), Firefox 90.0.2

Could not reproduce issue in either of these environments.

Note: not a valid test, as it was performed using the block editor.

Last edited 3 years ago by joedolson (previous) (diff)

#6 @benitolopez
3 years ago

I just tried it with Windows to make sure it wasn't related to a different OS. I'm still having the issue: Windows 10, WordPress 5.8 and Chrome (92.0.4515.107).

Same with Big Sur 11.5.1, WordPress 5.8 and Chrome (92.0.4515.107).

Again, this happens only when using the classic editor. If the post type uses the Block Editor, the featured image works correctly.

#7 follow-up: @joedolson
3 years ago

Ah; I followed the testing instructions, but didn't notice that the original ticket specified classic editor.

@hellofromTonya Can you edit the testing instructions to specify that the Classic Editor needs to be activated?

#8 in reply to: ↑ 7 @hellofromTonya
3 years ago

Replying to joedolson:

Ah; I followed the testing instructions, but didn't notice that the original ticket specified classic editor.

@hellofromTonya Can you edit the testing instructions to specify that the Classic Editor needs to be activated?

Thanks for the ping @joedolson! Testing report and steps are updated. Sorry for missing the information in the report 🤦‍♀️.

Retested with and without the Classic Editor plugin activated. Confirmed it only happens in the classic mode and not with the block editor. Testing report updated.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


3 years ago

#10 @joedolson
3 years ago

  • Owner set to joedolson
  • Status changed from new to accepted

#11 @peterwilsoncc
3 years ago

I did a bisect today, it appears to have been introduced in [50829].

While testing today, it happens in:

  • classic editor featured image (per image above)
  • classic editor inline image (click edit then replace buttons)
  • block editor classic block inline image (click edit then replace buttons)

With infinite scrolling enabled via add_filter( 'media_library_infinite_scrolling', '__return_true' ); the issue doesn't happen so I suspect the get-attachment action is confusing the logic and the load more button isn't displaying as it doesn't have the data.totalAttachments property.

I think when replacing an image there ought to be two initial ajax requests, one to get the selected attachment and one to get the first page of attachments via the query-attachments action.

#12 @circlecube
3 years ago

To be considered for the next release (5.8.1) this will need a patch and testing before RC1 which is scheduled for Wednesday, Sept 1.

#13 @desrosj
3 years ago

  • Milestone changed from 5.8.1 to 5.8.2

With 5.8.1 RC due out in less than 24 hours, I'm going to punt this one to 5.8.2.

#14 @danielbachhuber
3 years ago

I'm experiencing a similar issue in WordPress 5.8.1 (and all of the way down to WordPress 5.3?) It manifests in a couple different ways.

  • If you assign a featured image, close the modal, and then immediately re-open the featured image modal, no image is selected.
  • If you assign a featured image, close the modal, save the post, refresh the page, assign a new featured image, close the modal, and then immediately re-open the modal, the prior featured image will appear as selected.

Here's a Loom video documenting the behavior: https://www.loom.com/share/38df2347bd5648d2ac13c2904f55b301

My two problems happen with the Classic Editor plugin activated and deactivated.

#15 @PieWP
3 years ago

@joedolson not sure wether it helps but prior to founding out it was a core bug I encountered the following in debugging.

wp-includes/media-views.js:2410, this.collection.length returns 1 therefore the this.collection.more() ( line 2412 ) is no longer called and no additional media is being loaded.

Happy to see the workaround by @peterwilsoncc is working in the meantime.

#16 @desrosj
3 years ago

  • Milestone changed from 5.8.2 to Future Release

This one still needs a patch, and 5.8.2 RC is due out tomorrow. Moving to Future Release.

#17 @sabernhardt
3 years ago

#54403 was marked as a duplicate.

#18 @sabernhardt
3 years ago

  • Focuses javascript added

@szaqal21 forces loading more images on initialization, as a workaround (#54403). Could that help here?

This ticket was mentioned in Slack in #core-media by sabernhardt. View the logs.


3 years ago

#20 @joedolson
3 years ago

  • Milestone changed from Future Release to 5.9

#21 @szaqal21
3 years ago

I've made a lot of debug work and results are:

Gutenberg - lines from 487 to 493 of /wp-includes/js/dist/media-utils.js show what exactly is going on when frame opens, selection is being setup after attachments.more() is done

attachments.more().done(function () {
      var _attachments$models;

      if (isGallery && attachments !== null && attachments !== void 0 && (_attachments$models = attachments.models) !== null && _attachments$models !== void 0 && _attachments$models.length) {
        selection.add(attachments.models);
      }
    });

Classic editor - there is no call for loading more attachments on frame activation in /wp-includes/js/media-views.js lines 4047 - 4052

activate: function() {
		this.updateSelection();
		this.frame.on( 'open', this.updateSelection, this );

		Library.prototype.activate.apply( this, arguments );
	},

only updateSelection() (called twice, don't know why) which fetches only featured image (if set) so library has only one attachment and more() isn't called as @PieWP mentioned.

I've made those code changes:

includes/js/media-views.js lines 4047 - 4052 - call updateSelection() only on frame open event

activate: function() {
		this.frame.on( 'open', this.updateSelection, this );

		Library.prototype.activate.apply( this, arguments );
	},

added call of more() to updateSelection() in includes/js/media-views.js lines 4066 - 4077

updateSelection: function() {
		var selection = this.get('selection'),
			library = this.get('library'),
			id = wp.media.view.settings.post.featuredImageId,
			attachment;

		if ( '' !== id && -1 !== id ) {
			attachment = Attachment.get( id );
			attachment.fetch();
		}

		selection.reset( attachment ? [ attachment ] : [] );

		library.more();
	}

and includes/js/media-views.js lines 7470 - 7475

updateSelection: function() {
		var selection = this.get('selection'),
			library = this.get('library'),
			attachment = this.image.attachment;

		selection.reset( attachment ? [ attachment ] : [] );
		
		library.more();
	}

didn't noticed any unexpected behaviour in Gutenberg and Classic editor and everything works fine now. Perhaps it would be more elegant and intuitive to move more() call to activate() of ReplaceImage and FeatureImage views.

#22 @joedolson
3 years ago

Thanks for this work @szaqal21! This will be very helpful in moving this towards resolution.

@joedolson
3 years ago

Converts notes by szaqal21 into patch.

#23 @joedolson
3 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

These changes appear to work well in all relevant contexts. I added wrappers to verify library.hasMore() before calling .more(), but this patch is otherwise unchanged.

#24 @joedolson
3 years ago

  • Keywords commit added; needs-testing removed

#25 @joedolson
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 52167:

Media: Featured image modal loads only selected image.

Fix bug introduced in [50829] that caused media modal to only load the selected image. Executes .more() when loading the modal to ensure that the media collection is available.

Props benitolopez, hellofromTonya, peterwilsoncc, danielbachhuber, PieWP, sabernhardt, szaqal21.
Fixes #53765.

#26 @szaqal21
3 years ago

There's an update needed, after some more tests I've found some optimizations and one bug in my approach. wp.media.view.Attachments view calls scroll() on ready() if infinite scroll is enabled:

ready: function() {
		if ( this.options.infiniteScrolling ) {
			this.scroll();
		}
	},

so updateSelection() of wp.media.controller.ReplaceImage and wp.media.controller.FeaturedImage should be updated to conditionally call more() (only when infinite scroll if disabled):

FeaturedImage

updateSelection: function() {
		var selection = this.get('selection'),
			library = this.get('library'),
			id = wp.media.view.settings.post.featuredImageId,
			infinite_scrolling = wp.media.view.settings.infiniteScrolling,
			attachment;

		if ( '' !== id && -1 !== id ) {
			attachment = Attachment.get( id );
			attachment.fetch();
		}

		selection.reset( attachment ? [ attachment ] : [] );

		if ( ! infinite_scrolling && library.hasMore() ) {
			library.more();
		}
	}

and ReplaceImage

updateSelection: function() {
		var selection = this.get('selection'),
			library = this.get('library'),
			infinite_scrolling = wp.media.view.settings.infiniteScrolling,
			attachment = this.image.attachment;

		selection.reset( attachment ? [ attachment ] : [] );

		if ( ! infinite_scrolling && library.getTotalAttachments() == 0 && library.hasMore() ) {
			library.more();
		}
	}

there's library.getTotalAttachments() == 0 check in updateSelection() of ReplaceImage controller because activate() is being called every time user hits Replace button in edit image view if user would hit Replace button then hit Back and again hit Replace he would get another portion of attachments loaded.

#27 @hellofromTonya
3 years ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for #comment:26.

#28 @joedolson
3 years ago

@szagal21 Can you provide testing steps for replicating this issue? I'm not figuring out how to replicate the issue you're fixing. The changes seem totally reasonable, and they make sense, but without being able to confirm the bug and the fix, I have no way to demonstrate that it works.

I can confirm that it doesn't break anything new that I can see.

@joedolson
3 years ago

New patch

#29 @joedolson
3 years ago

  • Keywords reporter-feedback needs-testing added

New patch adds additional changes from @szaqal21

I haven't been able to identify a way to test these yet.

@szaqal21
3 years ago

Test scenario

#30 @joedolson
3 years ago

Thanks, @szaqal21!

Is this an accurate description of the issue?

1) Test prerequisites: a media library with more than 80 items.
2) Add an image to a post in the classic visual editor.
3) Click through to edit the image.
4) Choose the 'Replace' option. Scroll down and note 80 images with a load more button.
5) Exit the replace dialog.
6) Return to the Replace dialog. Scroll down and note all images loaded with no load more.

#31 @szaqal21
3 years ago

Yes exactly. If there would be more attachments (lets say 300) they would be loaded more and more (80 -> 160 -> 240 -> ...). This bug doesn't exist when posts_per_page is lower than 80, example attached with posts_per_page = 40

add_filter( 'ajax_query_attachments_args', 'change_posts_per_page', 10, 1 );

function change_posts_per_page($args)
{
	$args['posts_per_page'] = 40;
	
	return $args;
}

@szaqal21
3 years ago

posts_per_page = 40

@szaqal21
3 years ago

Noticed something else, lowering posts_per_page to 40 caused Load more button dissapear.

#32 @joedolson
3 years ago

Anything related to changing posts_per_page would be useful information on #53787; currently that's not expected to work correctly, because the media library uses a hard-coded value.

#33 @szaqal21
3 years ago

@joedolson thanks for clearing that out, lets forget about posts_per_page for now and focus on second fix it should be all fine now. Checking infitineScrolling prevents more() call which is already called by ready() method of Attachments view if infiniteScrolling is enabled:

ready: function() {
		if ( this.options.infiniteScrolling ) {
			this.scroll();
		}
	},

and checking if library.getTotalAttachments() equals 0 (when Replace image view is activated) ensures that more() is called only once when library is empty.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#35 @dariak
3 years ago

Test Report

Env

  • MacOS Big Sur
  • Chrome 96.0.4664.55
  • Localhost: Local
  • WordPress: 5.8.2 and 5.9-alpha-52250
  • Activated plugins: Classic Editor plugin and WordPress Beta Tester
  • Theme: Twenty Twenty One

Steps to test

  1. Use WordPress 5.8.2: add multiple images to the Media Library (128 in my case)
  2. Create a new post using Classic editor.
  3. Set the Featured Image.
  4. Save the post.
  5. Click on the image in the Featured image field to reopen the modal -> only selected image is shown.
  6. Upgrade to WordPress 5.9-alpha-52250.
  7. Re-open the same post from the step 2 in Classic editor.
  8. Click on the image in the Featured image field to reopen the modal -> first 80 items in the media library including the selected one are displayed.
  9. Create one more post using Classic editor.
  10. Set the Featured Image.
  11. Save the post.
  12. Click on the image in the Featured image field to reopen the modal -> first 80 items in the media library including the selected one are displayed.

So initial issue is fixed.

The "Load more" case currently works as follows:

  1. Create a new post using Classic editor.
  2. Add image to a post from Media Library (the Media Library has 128 images).
  3. Edit image.
  4. Click Replace -> first 80 images are shown, scroll down -> Load more button is shown.
  5. Click Back.
  6. Click Replace.

Indeed all images are shown without Load more button.

This ticket was mentioned in Slack in #core-test by dariak. View the logs.


3 years ago

#37 @szaqal21
3 years ago

@joedolson any commit comming for latest patch?

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


3 years ago

#40 @hellofromTonya
3 years ago

  • Keywords commit added; reporter-feedback needs-testing removed

Marking 53765.2.diff ready for commit.

#41 @hellofromTonya
3 years ago

  • Owner changed from joedolson to hellofromTonya
  • Status changed from reopened to reviewing

Per @joedolson in Slack earlier today:

No concerns I'm aware of; but I can't do it myself before beta 1.

Self-assigning for commit to include it in 5.9 Beta 1.

#42 @hellofromTonya
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52287:

Media: Featured image modal loads only selected image when infinite scroll is disabled.

Follow-up to [52167], which partially fixed a bug introduced in [50829] that caused media modal to only load the selected image.

This commit adds additional checks to ensure infinite scroll is disabled.

Follow-up to [50829], [52167].

Props dariak, joedolson, szaqal21.
Fixes #53765.

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 years ago

This ticket was mentioned in PR #1981 on WordPress/wordpress-develop by hellofromtonya.


3 years ago
#44

Follow-up to 52287 which brings the global infinite scroll setting into the functions and changes from snake_case to camelCase for consistency and JS coding standards.

Trac ticket: https://core.trac.wordpress.org/ticket/53765

#45 @hellofromTonya
3 years ago

  • Keywords commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address the undefined infinite_scrolling variable. PR 1981 brings the global setting into the respective functions and changes the variable name to camelCase.

#46 @hellofromTonya
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 52288:

Media: Use infiniteScrolling global setting in js/media/controllers/featured-image.js and js/media/controllers/replace-image.js.

Follow-up to [52287] which added an undefined variable. The variable should have been the global wp.media.view.settings.infiniteScrolling. This commit brings that global setting into each of the functions and renames the variable using camelCase to comply with JS coding standards.

Follow-up to [52287].

Props SergeyBiryukov.
Fixes #53765.

#47 @hellofromTonya
3 years ago

@szaqal21 can you test trunk to verify the changeset resolves the issue you noted? If no, please reopen this ticket.

#49 follow-up: @szaqal21
3 years ago

@hellofromTonya all issues (I've noticed before) are resolved by this changeset, perfect, thanks.

#50 in reply to: ↑ 49 @hellofromTonya
3 years ago

Replying to szaqal21:

@hellofromTonya all issues (I've noticed before) are resolved by this changeset, perfect, thanks.

Oh that's good news. Thank you @szaqal21 for confirming.

#51 @szaqal21
3 years ago

Yesterday I was testing my plugin which adds ability to add second featured image to post (written based on WP 5.8 code), my controller:

var FeaturedImage2 = wp.media.controller.FeaturedImage.extend(
	{
		defaults: _.defaults({
			id: "featured-image2"
		}, wp.media.controller.Library.prototype.defaults ),
		
		updateSelection : function() {
			var selection = this.get("selection"),
			id = wp.media.view.settings.post.featuredImage2Id,
			attachment;

			if ( "" !== id && -1 !== id ) {
				attachment = wp.media.model.Attachment.get( id );
				attachment.fetch();
			}

			selection.reset( attachment ? [ attachment ] : [] );
		}
	});

as you can see I've overriden updateSelection() method of FeaturedImage controller so it doesn't contain newset patch from WP 5.9 Beta 1

if ( ! infiniteScrolling && library.getTotalAttachments() === 0 && library.hasMore() ) {
			library.more();
		}

to my surprise it worked fine, all attachments were loaded when trying to change featured image (not only selected one). It turns out that removal of updateSelection() from activate() method of FeaturedImage controller in first patch https://core.trac.wordpress.org/attachment/ticket/53765/53765.diff already fixed the issue in this case. There is no need to conditionally call library.more() in updateSelection() because AttachmentsBrowser view already calls more() in updateContent() method when initializing. So I've reverted changes in updateSelection() (removed library, infiniteScrolling vars and conditional):

/**
	 * @since 3.5.0
	 */
	activate: function() {
		this.frame.on( 'open', this.updateSelection, this );

		Library.prototype.activate.apply( this, arguments );
	},

	/**
	 * @since 3.5.0
	 */
	deactivate: function() {
		this.frame.off( 'open', this.updateSelection, this );

		Library.prototype.deactivate.apply( this, arguments );
	},

	/**
	 * @since 3.5.0
	 */
	updateSelection: function() {
		var selection = this.get('selection'),
			id = wp.media.view.settings.post.featuredImageId,
			attachment;

		if ( '' !== id && -1 !== id ) {
			attachment = Attachment.get( id );
			attachment.fetch();
		}

		selection.reset( attachment ? [ attachment ] : [] );
	}

and all tests went fine.

I've also updated ReplaceImage controller to call updateSelection() on frame event, it delays call of updateSelection() to be called after view initialization (not sure if this event is the best choice but I couldn't find any better):

/**
	 * @since 3.9.0
	 */
	activate: function() {
		this.frame.on( 'content:render:browse', this.updateSelection, this );
		
		Library.prototype.activate.apply( this, arguments );
	},
	
	/**
	 * @since 5.9.0
	 */
	deactivate: function() {
		this.frame.off( 'content:render:browse', this.updateSelection, this );
		
		Library.prototype.deactivate.apply( this, arguments );
	},

	/**
	 * @since 3.9.0
	 */
	updateSelection: function() {
		var selection = this.get('selection'),
			attachment = this.image.attachment;

		selection.reset( attachment ? [ attachment ] : [] );
	}

@joedolson could you test the changes?

@hellofromTonya @joedolson what do you think, shall we reopen it?

Last edited 3 years ago by szaqal21 (previous) (diff)

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


3 years ago

@szaqal21
3 years ago

Bug in Featured Image dialog.

#53 @szaqal21
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Current fix still contains a bug in Featured Image dialog, I've proposed necessary changes here https://core.trac.wordpress.org/ticket/53765#comment:51.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 years ago

This ticket was mentioned in PR #2039 on WordPress/wordpress-develop by hellofromtonya.


3 years ago
#55

Reverts [52287-52288]
Partial revert of [52167] - reverts the library.more() logic

Trac ticket: https://core.trac.wordpress.org/ticket/53765

#56 follow-up: @hellofromTonya
3 years ago

@benitolopez can you test PR 2039 please?

#57 in reply to: ↑ 56 ; follow-up: @benitolopez
3 years ago

Replying to hellofromTonya:

@benitolopez can you test PR 2039 please?

Sorry for the late reply. Yes, I confirm that the issue is solved on my end using WordPress 5.9 Beta 2!

#58 in reply to: ↑ 57 @hellofromTonya
3 years ago

Replying to benitolopez:

Sorry for the late reply. Yes, I confirm that the issue is solved on my end using WordPress 5.9 Beta 2!

w00t, thank you for your confirming!

@szaqal21 can you test PR 2039? Does it resolve the issue you reopened the ticket for?

#59 @szaqal21
3 years ago

@hellofromTonya sorry but I can't test PR because I'm using WordPress Beta Tester plugin and latest version I can test is Trunk. If you could give me some hints how to test PR I'll give it a tray right away. Code seems 100% ok but I want to be sure this time.

#60 follow-up: @szaqal21
3 years ago

@hellofromTonya I've managed to test your PR and it resolves the issue that I've reopened the ticket for.

#61 in reply to: ↑ 60 @hellofromTonya
3 years ago

Replying to szaqal21:

I've managed to test your PR and it resolves the issue that I've reopened the ticket for.

That's awesome news! Thank you for testing it @szaqal21

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


3 years ago

#63 @hellofromTonya
3 years ago

  • Keywords commit added
  • Owner hellofromTonya deleted
  • Status changed from reopened to reviewing

Marking PR 2039 for commit, pending feedback from @joedolson. Removing myself from owner to way for any committer to review and commit.

#64 @hellofromTonya
3 years ago

  • Keywords commit removed

Removing commit keyword. @joedolson is unavailable today, but plans to review and follow-up before Beta 4 (i.e. Dec 21st).

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-media by hellofromtonya. View the logs.


3 years ago

#68 @hellofromTonya
3 years ago

@szaqal21 is the deactivate: function() in the ReplaceImage controller needed? Not seeing where it's used in Core.

#69 @szaqal21
3 years ago

@hellofromTonya yes, previously updateSelection() was called strict in activate() of ReplaceImage controller, now it is called on event and handler is being removed in deactivate() this is why deactivate() is needed (same approach as in FeaturedImage activate() and deactivate()):

/**
	 * @since 3.5.0
	 */
	activate: function() {
		this.frame.on( 'open', this.updateSelection, this );

		Library.prototype.activate.apply( this, arguments );
	},

	/**
	 * @since 3.5.0
	 */
	deactivate: function() {
		this.frame.off( 'open', this.updateSelection, this );

		Library.prototype.deactivate.apply( this, arguments );
	},
Last edited 3 years ago by szaqal21 (previous) (diff)

#70 @hellofromTonya
3 years ago

  • Keywords commit added

Marking for commit.

#71 @antpb
3 years ago

  • Owner set to antpb
  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52384:

Media: Fix selections in Media Library Featured Image modal on open.

In [50829] infinite scrolling was removed from the Media Library and modal which introduced unintended behavior for featured images where only the selected image shows when opening the library. This change reverts only the logic that caused this and applies a proper fix when opening the library.

Props benitolopez, hellofromTonya, joedolson, peterwilsoncc, circlecube, danielbachhuber, PieWP, sabernhardt, szaqal21, dariak, sergeybiryukov.
Fixes #53765.

#73 @hellofromTonya
2 years ago

#55221 was marked as a duplicate.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


21 months ago

Note: See TracTickets for help on using tickets.