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

Gallery block / media-placeholder - Preserve changes made while upload in progress. #19134

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
256a2d1
added optional prop to pass by reference instead of value
Addison-Stavlo Dec 12, 2019
6b23087
currentValue updated on adding/removing
Addison-Stavlo Dec 12, 2019
2c450bf
comment update
Addison-Stavlo Dec 12, 2019
16b589f
fixed multiple upload bug
Addison-Stavlo Dec 12, 2019
86ed836
removed stuff for trying to keep loaders that werent part of the uplo…
Addison-Stavlo Dec 13, 2019
14fff3a
updates comments and naming
Addison-Stavlo Dec 13, 2019
bce4177
updated galleryValue to fallback to empty array
Addison-Stavlo Dec 13, 2019
7c22bc8
renamed values to be more semantic
Addison-Stavlo Dec 17, 2019
794f9e8
removed dynamicAlteration prop
Addison-Stavlo Dec 18, 2019
30d884c
refactored to compare last pass of newMedia
Addison-Stavlo Jan 3, 2020
50ed3ad
added comments, changed variable name
Addison-Stavlo Jan 6, 2020
9f7e10f
simplified setting lastMediaPassed
Addison-Stavlo Jan 6, 2020
5a3cfd9
compare via includes instead
Addison-Stavlo Jan 15, 2020
ea1accd
added id check as first priority in filter
Addison-Stavlo Jan 27, 2020
7aff93f
edited comments
Addison-Stavlo Jan 27, 2020
102a3c2
rebased
Addison-Stavlo Mar 11, 2020
90d1e24
changed to soft equal for int/string ID comparison
Addison-Stavlo Mar 11, 2020
9a77709
added unchanging reference for image array
Addison-Stavlo Mar 11, 2020
aa2230a
re-added fallback value for currentMedia, fixed linter == problem
Addison-Stavlo Apr 17, 2020
3d02620
updated comment
Addison-Stavlo Apr 17, 2020
d234546
re-added addToGallery check, forced 'true' for our gallery
Addison-Stavlo Apr 17, 2020
b651819
refactored to use a getValue instead
Addison-Stavlo Apr 25, 2020
df3c9ce
updated comment
Addison-Stavlo Apr 25, 2020
d89cea8
Remove getValue and use a reference to this.props.value
jorgefilipecosta May 7, 2020
8d314ac
comment update
Addison-Stavlo May 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions packages/block-editor/src/components/media-placeholder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,50 @@ export class MediaPlaceholder extends Component {
multiple,
onError,
onSelect,
value = [],
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
} = this.props;
let setMedia;
if ( multiple ) {
if ( addToGallery ) {
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
const currentValue = value;
// To allow changes to a gallery to be made while uploads are in progress
// (including trigging multiple upload groups and removing already in place images),
// we must be able to add newMedia based on the current value of the Gallery
// whenever the setMedia function runs (not destructuring 'value' from props).
// Additionally, since the setMedia function runs multiple times per upload group
// and is passed newMedia containing every item in its group each time, we must
// also filter out whatever this upload group had previously returned to the
// gallery before adding and returning the image array with replacement newMedia
// values.

// Define an array to store urls from newMedia between subsequent function calls.
let lastMediaPassed = [];
setMedia = ( newMedia ) => {
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
onSelect( currentValue.concat( newMedia ) );
// Remove any images this upload group is responsible for (lastMediaPassed).
// Their replacements are contained in newMedia.
const filteredMedia = ( this.props.value || [] ).filter(
( item ) => {
// If Item has id, only remove it if lastMediaPassed has an item with that id.
if ( item.id ) {
return ! lastMediaPassed.some(
// Be sure to convert to number for comparison.
( { id } ) =>
Number( id ) === Number( item.id )
);
}
// Compare transient images via .includes since gallery may append extra info onto the url.
return ! lastMediaPassed.some( ( { urlSlug } ) =>
item.url.includes( urlSlug )
);
}
);
// Return the filtered media array along with newMedia.
onSelect( filteredMedia.concat( newMedia ) );
// Reset lastMediaPassed and set it with ids and urls from newMedia.
lastMediaPassed = newMedia.map( ( media ) => {
// Add everything up to '.fileType' to compare via .includes.
const cutOffIndex = media.url.lastIndexOf( '.' );
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
const urlSlug = media.url.slice( 0, cutOffIndex );
return { id: media.id, urlSlug };
} );
};
} else {
setMedia = onSelect;
Expand Down
5 changes: 2 additions & 3 deletions packages/block-library/src/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,10 @@ class GalleryEdit extends Component {
} = attributes;

const hasImages = !! images.length;
const hasImagesWithId = hasImages && some( images, ( { id } ) => id );

const mediaPlaceholder = (
<MediaPlaceholder
addToGallery={ hasImagesWithId }
addToGallery={ true }
Copy link
Contributor

@talldan talldan May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is the cause of #22338, that when first adding a gallery block the Media Library is always in addToGallery mode with all images selected on the Insert Gallery page.

Copy link
Contributor

@talldan talldan May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've read more of the details on this PR, and I see that the issue is related to hasImagesWithId being false when an image upload is in progress and other operations causing the uploading image to be replaced.

I've made naive PR at #22659 to attempt to solve #22338

isAppender={ hasImages }
className={ className }
disableMediaButtons={ hasImages && ! isSelected }
Expand All @@ -372,7 +371,7 @@ class GalleryEdit extends Component {
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
multiple
value={ hasImagesWithId ? images : undefined }
value={ images }
onError={ this.onUploadError }
notices={ hasImages ? undefined : noticeUI }
onFocus={ this.props.onFocus }
Expand Down