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

Update on Gallery Block. Replaced FormFileUpload with MediaPlaceholder. #9682

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 30 additions & 15 deletions packages/block-library/src/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { __ } from '@wordpress/i18n';
import {
IconButton,
DropZone,
FormFileUpload,
PanelBody,
RangeControl,
SelectControl,
Expand Down Expand Up @@ -55,7 +54,8 @@ class GalleryEdit extends Component {
this.onRemoveImage = this.onRemoveImage.bind( this );
this.setImageAttributes = this.setImageAttributes.bind( this );
this.addFiles = this.addFiles.bind( this );
this.uploadFromFiles = this.uploadFromFiles.bind( this );
this.onAddMoreImages = this.onAddMoreImages.bind( this );
this.updateImagesAttribute = this.updateImagesAttribute.bind( this );

this.state = {
selectedImage: null,
Expand Down Expand Up @@ -123,10 +123,6 @@ class GalleryEdit extends Component {
} );
}

uploadFromFiles( event ) {
this.addFiles( event.target.files );
}

addFiles( files ) {
const currentImages = this.props.attributes.images || [];
const { noticeOperations, setAttributes } = this.props;
Expand All @@ -142,6 +138,21 @@ class GalleryEdit extends Component {
} );
}

onAddMoreImages( newImages ) {
if ( ! window.event ) { // from "Upload" option after newImages absolute paths have been generated
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not certain I understand why do we need this if else and I don't think having two separate functions is really needed here unless I'm missing something?

Copy link
Author

Choose a reason for hiding this comment

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

@youknowriad thanks for the feedback.

Please see response to your questions below;

1. why do we need this if else
Without the if else, when one adds an image to the gallery using the MediaPlaceholder "Upload" button, 2 images are added instead of 1 causing duplicity.
We noticed this was because, during the file upload 2 steps happen. Step1: A temporary path is created. Step2: An absolute path is created after image is sucessfully uploaded. These 2 paths were both being picked and added to the gallery.
To stop the duplicity we used the if else to confirm that the upload event is complete and that's when we pick the new image's absolute path and add it to the gallery.

2. I don't think having two separate functions is really needed here unless I'm missing something?
We used two separate functions onAddMoreImages() and updateImagesAttribute() for code readability and ease debugging. Also in the future, one might want to call updateImagesAttribute() without onAddMoreImages().

this.updateImagesAttribute( newImages );
} else if ( window.event.type === 'click' ) { // from "Media Library" option
this.updateImagesAttribute( newImages );
}
}

updateImagesAttribute( newImages ) {
const currentImages = this.props.attributes.images || [];
this.props.setAttributes( {
images: currentImages.concat( newImages.map( ( image ) => pick( image, [ 'url', 'link', 'alt', 'id', 'caption' ] ) ) ),
} );
}

componentDidUpdate( prevProps ) {
// Deselect images when deselecting the block
if ( ! this.props.isSelected && prevProps.isSelected ) {
Expand Down Expand Up @@ -253,16 +264,20 @@ class GalleryEdit extends Component {
) ) }
{ isSelected &&
<li className="blocks-gallery-item has-add-item-button">
<FormFileUpload
multiple
isLarge
className="block-library-gallery-add-item-button"
onChange={ this.uploadFromFiles }
<MediaPlaceholder
icon="format-gallery"
className={ className }
labels={ {
title: __( 'Gallery' ),
name: __( 'images' ),
} }
onSelect={ this.onAddMoreImages }
accept="image/*"
icon="insert"
>
{ __( 'Upload an image' ) }
</FormFileUpload>
type="*"
multiple
notices={ noticeUI }
onError={ noticeOperations.createErrorNotice }
/>
</li>
}
</ul>
Expand Down
25 changes: 0 additions & 25 deletions packages/block-library/src/gallery/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -53,31 +53,6 @@
}
}

.components-form-file-upload,
.components-button.block-library-gallery-add-item-button {
width: 100%;
height: 100%;
}

.components-button.block-library-gallery-add-item-button {
display: flex;
flex-direction: column;
justify-content: center;
box-shadow: none;
border: none;
border-radius: 0;
min-height: 100px;

& .dashicon {
margin-top: 10px;
}

&:hover,
&:focus {
border: $border-width solid $dark-gray-500;
}
}

.editor-rich-text .editor-rich-text__tinymce {
a {
color: $white;
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/media-upload/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
MediaUpload
===========

MediaUpload is a React component used to render a button that opens a the WordPress media modal.
MediaUpload is a React component used to render a button that opens the WordPress media modal.

## Setup

Expand Down