Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media Library doesn't update after dragging image onto Image block #4612

Closed
maddisondesigns opened this issue Jan 20, 2018 · 12 comments

Comments

@maddisondesigns
Copy link

commented Jan 20, 2018

Issue Overview

After inserting an Image block, I then dragged 'n dropped an image directly onto the block. I then inserted another image block and after clicking the Add from Media Library button, and displaying the Media popup window, the previous image that I added to the page isn't displaying in the Media Library.

New image that I dragged onto Image Block
gutenberg-imgblockdragdrop

Newly added image not appearing in Media LIbrary
gutenberg-imgnotinlibrary

Steps to Reproduce (for bugs)

  1. Insert Image block
  2. Drag 'n drop image onto Image block
  3. Insert new Image block
  4. Click the Add from Media Library button and try to add the same image. You can't, because it doesn't appear in the Media Library

Expected Behavior

If I drag'n drop an image onto an Image BLock, this should get added to my Media LIbraray, and should be displayed when I try to add another image by viewing the Media Library popup

Current Behavior

Images dragged onto Image Block aren't reflected in Media Library popup window

Firefox Quantum 57.0.4 (64-bit)
Gutenberg 2.0.0
WordPress 4.9.2
No other plugins activated

@karmatosed

This comment has been minimized.

Copy link
Member

commented Jan 22, 2018

Just checking the steps here as can't reproduce.

I added an image via drag and drop:

2018-01-22 at 16 53

I then clicked image block and to see if it's in the media library:

2018-01-22 at 16 52

@maddisondesigns

This comment has been minimized.

Copy link
Author

commented Jan 23, 2018

I can consistently replicate the issue by;

  • Insert 1st Image Block
  • Drag image to 1st block
  • Insert 2nd Image Block
  • Click Add from Media Library button on 2nd block (The first image you dragged will most likely show)
  • Close Popup window
  • Drag new image to 2nd block
  • Insert 3rd Image block
  • Click Add from Media Library button on new block (Image that was last dragged doesn't appear)

gutenberg-dragdropimageblock2

@karmatosed

This comment has been minimized.

Copy link
Member

commented Jan 23, 2018

Ah so there were some missing steps there. I can replicate following exactly those steps thanks. It's also in Chrome, so I don't think a browser issue.

@karmatosed karmatosed added this to the Merge Proposal milestone Jan 25, 2018

@jeffpaul

This comment has been minimized.

Copy link
Member

commented Mar 15, 2018

This ticket was mentioned in Slack in #core-editor by jeffpaul. View the logs.

@jeffpaul

This comment has been minimized.

Copy link
Member

commented Mar 15, 2018

This ticket was mentioned in Slack in #core-editor by jeffpaul. View the logs.

@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Media Apr 12, 2018

@jorgefilipecosta jorgefilipecosta self-assigned this Apr 24, 2018

@mtias mtias modified the milestones: Merge: Media, WordPress 5.0 Oct 3, 2018

@antpb

This comment has been minimized.

Copy link
Contributor

commented Oct 8, 2018

The latest commit from #6390 does fix the issue. It would need to be tested and possibly optimized. From my end, it seems to be working well but there may be some edge cases we're missing.

Mentioned by @jorgefilipecosta,

"in order to fix the bug we need to request all images each time we open the media library (to know if new media was added in parallel) but that makes the open of the media library slower, it is a tradeoff that we should make."

@joemcgill, we will be digging into backbone things soon. Want to get this PR on your radar if you have the bandwidth to review. If you have any thoughts on why we shouldn't go the proposed route, I'd love to hear them.

Based on the above info, Is this a performance hit we'd be willing to take and improve on later to fix the bug for 5.0? Is there another option?

@joemcgill joemcgill self-assigned this Oct 11, 2018

@joemcgill

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

Fetching a new collection each time the modal is opened is an ok approach, but I would prefer to see us try an approach where we subscribe to an event that fires any time an item is uploaded and fetch only when that event occurs.

@antpb

This comment has been minimized.

Copy link
Contributor

commented Oct 20, 2018

@joemcgill I moved the approach from that PR to the mediaUpload function. This seems to work from my end and doesn't require us to load the collections every time we open the modal. Agree?

Actually, I think this is no longer an issue. I am unable to reproduce now.

@aduth aduth added the Needs Testing label Oct 23, 2018

@antpb

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

@afercia was able to reproduce using the upload button (thank you!) I was able to reproduce using the upload button. I've re-opened the PR above that refreshes collection when the media library is opened. I think we should go this route and improve later. I could open an issue after we merge to optimize the collections of the Media library based on the feedback provided above.
Link to PR: #10843

@designsimply

This comment has been minimized.

Copy link
Contributor

commented Oct 24, 2018

Tested and confirmed the bug using WordPress 4.9.8 and Gutenberg 4.1.0-rc.2 with Firefox 62.0.3 on macOS 10.13.6. (53s)

I noticed there is a suggested improvement on #10843 so I'll take away the Needs Testing label for now and if you need help testing the PR once it's ready-to-go, feel free to ping me here or ping in #core-test on WP Slack to ask for testing help!

@jorgefilipecosta

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

I think closing this issue will also close #11567.

@swissspidy

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

@jorgefilipecosta Indeed my issue sounds like a duplicate of this one.

gziolo added a commit that referenced this issue Nov 19, 2018

Fix/media library collections #4612 (#10843)
*  removes destroyCollection and moves refresh to `onOpen`

* Removes file saved from vim-ing/rebasing weird. :)

* remove leftovers from rebase

youknowriad added a commit that referenced this issue Nov 19, 2018

@youknowriad youknowriad reopened this Nov 19, 2018

youknowriad added a commit that referenced this issue Nov 19, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this issue Nov 22, 2018

Fix/media library collections WordPress#4612 (WordPress#10843)
*  removes destroyCollection and moves refresh to `onOpen`

* Removes file saved from vim-ing/rebasing weird. :)

* remove leftovers from rebase

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this issue Nov 22, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this issue Jan 4, 2019

Fix/media library collections WordPress#4612 (WordPress#10843)
*  removes destroyCollection and moves refresh to `onOpen`

* Removes file saved from vim-ing/rebasing weird. :)

* remove leftovers from rebase

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this issue Jan 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.