-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
MediaUpload: Ensure current images in a gallery are selected after opening media library #35070
MediaUpload: Ensure current images in a gallery are selected after opening media library #35070
Conversation
…ening media library
Size Change: +38 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
…h non-image based media
@talldan and @jorgefilipecosta — I've added you both as reviewers, but mostly just as an FYI since I saw you've both worked on this file before 🙂 |
Excellent work figuring this out @andrewserong 🎉 ✅ Works as described in both current and refactored galleries: clicking on the Media Library button shows Edit gallery window with the current gallery images selected. One thing I noticed, and I don't know if it's related to this PR was that sometimes if you select "Edit gallery" on the left hand side of the upload modal, then close the modal, when you open up the media library again the "Edit gallery" is showing. When you switch to the "Add to gallery" pane, it only shows the images that are not currently in the gallery with the caption Sep-23-2021.19-53-44.mp4Should we be defaulting to the "Add to gallery" pane when opening the upload modal from the editor? Seeing as we can already edit the gallery within the editor? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tested well for me with the gallery, stand alone image block, cover block.
I think this would be a good fix to get out as soon as possible, and then look at a way of making the UX the same when first loading and when flipping between edit and add, ie. in both instances in the add screen only show images not already in the gallery, but fixing the very broken current experience is the important thing for starters.
This would make sense to me, but the way it works currently I think is deliberate behaviour in core in that it caches the previous state of that model - not sure if we can influence that from the editor side or not, but might be worth exploring in a separate issue. |
I confirm that this bug causes gallery blocks are unintentionally emptied when editors just want to add more pictures. |
Thanks for the reviews, folks! I'll merge this in, since it resolves the issue of the media library modal unintentionally stripping existing images from the gallery block.
I agree, let's explore fine-tuning the UX in follow-ups 👍. I'll create a follow-up issue to keep track of it. |
Follow-up issue for looking into consistency of the "Add to gallery" view added here: #35130 |
Description
Fixes #33769
Before this change, when we open the media library for a gallery with existing images, in the media library's Add to Gallery view, the current images are not selected, and the state for the edit controller isn't loaded yet, which means that the gallery is treated as empty. Adding any images at this point effectively replaces all images in the gallery.
In this fix, to workaround that the
GalleryAdd
controller watches the gallery-edit state to determine which attachments are already in the gallery, we explicitly set the current images in the gallery to be selected, once they're loaded.This means that when the user opens the media library, they will see the current images selected in addition to other images in their library. If they click to the Edit view and then back to Add to Gallery, the existing behaviour where the current images are hidden will persist, after all this is just a workaround! But on balance, it should mean that people don't unexpectedly wind up removing images from their image galleries.
This fix preserves the optimisation introduced back in #2488, so shouldn't result in any additional API calls.
How has this been tested?
Manually:
Also test other media library behaviour in the gallery, image, and video blocks to ensure there are no regressions.
Screenshots
In the before screengrab, no images are selected and clicking on one effectively replaces the entire gallery. In the after screengrab, the current images are pre-selected so clicking on an additional image adds it to the gallery.
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.js
files for terms that need renaming or removal).