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

Add Gallery Block #740

Merged
merged 19 commits into from
Jun 9, 2017
Merged
Show file tree
Hide file tree
Changes from 15 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
10 changes: 10 additions & 0 deletions blocks/library/gallery/gallery-image.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

export default class GalleryImage extends wp.element.Component {
Copy link
Member

Choose a reason for hiding this comment

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

Dos this need to be an element.Component or could it be just a function?

render() {
return (
<div key={ this.props.i } className="blocks-gallery-image">
Copy link
Member

Choose a reason for hiding this comment

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

key here isn't doing anything.

Copy link
Member

Choose a reason for hiding this comment

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

This wasn't addressed.

<img src={ this.props.img.url } alt={ this.props.img.alt } />
</div>
);
}
}
142 changes: 142 additions & 0 deletions blocks/library/gallery/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* Internal dependencies
*/
import './style.scss';
import { registerBlockType, query as hpq } from '../../api';

import Placeholder from 'components/placeholder';
import MediaUploadButton from '../../media-upload-button';

import GalleryImage from './gallery-image';

const { query, attr } = hpq;

const editMediaLibrary = ( attributes, setAttributes ) => {
const frameConfig = {
frame: 'post',
title: wp.i18n.__( 'Update Gallery media' ),
button: {
text: wp.i18n.__( 'Select' ),
},
multiple: true,
state: 'gallery-edit',
selection: new wp.media.model.Selection( attributes.images, { multiple: true } ),
};

const editFrame = wp.media( frameConfig );
function updateFn() {
setAttributes( {
images: this.frame.state().attributes.library.models.map( ( a ) => {
return a.attributes;
} ),
} );
}

editFrame.on( 'insert', updateFn );
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to detach these event handlers after the edit frame is closed, to avoid risk of zombie event listeners?

Copy link
Member Author

Choose a reason for hiding this comment

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

On the frame Unmount the entire frame is destroyed, so the event handlers should be taken care of with that. See https://github.com/WordPress/gutenberg/blob/master/blocks/media-upload-button/index.js#L36

editFrame.state( 'gallery-edit' ).on( 'update', updateFn );
editFrame.open( 'gutenberg-gallery' );
};

/**
* Returns an attribute setter with behavior that if the target value is
* already the assigned attribute value, it will be set to undefined.
*
* @param {string} align Alignment value
* @return {Function} Attribute setter
*/
function toggleAlignment( align ) {
return ( attributes, setAttributes ) => {
const nextAlign = attributes.align === align ? undefined : align;
setAttributes( { align: nextAlign } );
};
}

registerBlockType( 'core/gallery', {
title: wp.i18n.__( 'Gallery' ),
icon: 'format-gallery',
category: 'common',

attributes: {
images:
query( 'div.blocks-gallery div.blocks-gallery-image img', {
url: attr( 'src' ),
alt: attr( 'alt' ),
} ),
},

controls: [
{
icon: 'format-image',
title: wp.i18n.__( 'Edit Gallery' ),
onClick: editMediaLibrary,
},
{
icon: 'align-left',
title: wp.i18n.__( 'Align left' ),
isActive: ( { align } ) => 'left' === align,
onClick: toggleAlignment( 'left' ),
},
{
icon: 'align-center',
title: wp.i18n.__( 'Align center' ),
isActive: ( { align } ) => ! align || 'center' === align,
onClick: toggleAlignment( 'center' ),
},
{
icon: 'align-right',
title: wp.i18n.__( 'Align right' ),
isActive: ( { align } ) => 'right' === align,
onClick: toggleAlignment( 'right' ),
},
{
icon: 'align-full-width',
title: wp.i18n.__( 'Wide width' ),
isActive: ( { align } ) => 'wide' === align,
onClick: toggleAlignment( 'wide' ),
},
],

edit( { attributes, setAttributes } ) {
const { images, align = 'none' } = attributes;
if ( ! images ) {
const setMediaUrl = ( imgs ) => setAttributes( { images: imgs } );
return (
<Placeholder
instructions={ wp.i18n.__( 'Drag images here or insert from media library' ) }
icon="format-gallery"
label={ wp.i18n.__( 'Gallery' ) }
className="blocks-gallery">
<MediaUploadButton
onSelect={ setMediaUrl }
type="image"
auto-open
Copy link
Member

Choose a reason for hiding this comment

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

Curious why we chose dash-separated here, instead of autoOpen. Camel-case to be the convention with prop naming.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how it was specified in MediaUploadButton, I'd prefer to ship and then fix in another PR since it touches a couple blocks at once. Created issue #1083 to handle

multiple="true"
>
{ wp.i18n.__( 'Insert from Media Library' ) }
</MediaUploadButton>
</Placeholder>
);
}

return (
<div className={ `blocks-gallery align${ align }` }>
{ images.map( ( img, i ) => (
<GalleryImage key={ i } img={ img } />
) ) }
</div>
);
},

save( { attributes } ) {
const { images, align = 'none' } = attributes;

return (
<div className={ `blocks-gallery align${ align }` } >
{ images.map( ( img, i ) => (
<GalleryImage key={ i } img={ img } />
) ) }
</div>
);
},

} );
37 changes: 37 additions & 0 deletions blocks/library/gallery/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@

.blocks-gallery {

display: flex;
flex-wrap: wrap;

.blocks-gallery-image {

margin: 8px;

img {
max-width: 120px;
}
}
}

.blocks-gallery.is-placeholder {
margin: -15px;
padding: 6em 0;
border: 2px solid $light-gray-500;
text-align: center;
}

.blocks-gallery__placeholder-label {
display: flex;
align-items: center;
justify-content: center;
font-weight: bold;

.dashicon {
margin-right: 1ch;
}
}

.blocks-gallery__placeholder-instructions {
margin: 1.8em 0;
}
1 change: 1 addition & 0 deletions blocks/library/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ import './pullquote';
import './table';
import './preformatted';
import './code';
import './gallery';
10 changes: 10 additions & 0 deletions blocks/test/fixtures/core-gallery.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!-- wp:core/gallery -->
<div class="blocks-gallery">
<div class="blocks-gallery-image">
Copy link
Member

Choose a reason for hiding this comment

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

These could be figure instead of div for the markup.

<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</div>
<div class="blocks-gallery-image">
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</div>
</div>
<!-- /wp:core/gallery -->
12 changes: 12 additions & 0 deletions blocks/test/fixtures/core-gallery.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"uid": "_uid_0",
"name": "core/gallery",
"attributes": {
"images": [
{ "url": "https://cldup.com/uuUqE_dXzy.jpg", "alt": "title" },
{ "url": "https://cldup.com/uuUqE_dXzy.jpg", "alt": "title" }
]
}
}
]
7 changes: 7 additions & 0 deletions blocks/test/fixtures/core-gallery.serialized.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<!-- wp:core/gallery -->
<div class="blocks-gallery alignnone">
<div class="blocks-gallery-image"><img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" /></div>
<div class="blocks-gallery-image"><img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" /></div>
</div>
<!-- /wp:core/gallery -->

5 changes: 4 additions & 1 deletion blocks/test/full-content.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ function normalizeReactTree( element ) {
return element.map( child => normalizeReactTree( child ) );
}

if ( isObject( element ) ) {
// Check if we got an object first, then if it actually has a `type` like a
// React component. Sometimes we get other stuff here, which probably
// indicates a bug.
if ( isObject( element ) && element.type ) {
const toReturn = {
type: element.type,
};
Expand Down