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.

Sign up for GitHub

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

Try new gallery markup #3996

Merged
merged 6 commits into from
Jan 18, 2018
Merged

Try new gallery markup #3996

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d8eaefb
Try new gallery markup
Dec 14, 2017
4466445
Address feedback
Jan 18, 2018
3da0798
Replace all instances of blocks-gallery-image
Jan 18, 2018
450f716
Try adjusting deprecated attributes
Jan 18, 2018
91b2504
Gallery Block: Fix deprecated version
youknowriad Jan 18, 2018
d04b78d
Gallery Block: Fix tests
youknowriad Jan 18, 2018
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
25 changes: 13 additions & 12 deletions blocks/library/gallery/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,21 +235,22 @@ class GalleryBlock extends Component {
/>
</InspectorControls>
),
<div key="gallery" className={ `${ className } align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` }>
<ul key="gallery" className={ `${ className } align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` }>
{ dropZone }
{ images.map( ( img, index ) => (
<GalleryImage
key={ img.id || img.url }
url={ img.url }
alt={ img.alt }
id={ img.id }
isSelected={ this.state.selectedImage === index }
onRemove={ this.onRemoveImage( index ) }
onClick={ this.onSelectImage( index ) }
setAttributes={ ( attrs ) => this.setImageAttributes( index, attrs ) }
/>
<li className="blocks-gallery-item" key={ img.id || img.url }>
<GalleryImage
url={ img.url }
alt={ img.alt }
id={ img.id }
isSelected={ this.state.selectedImage === index }
onRemove={ this.onRemoveImage( index ) }
onClick={ this.onSelectImage( index ) }
setAttributes={ ( attrs ) => this.setImageAttributes( index, attrs ) }
/>
</li>
) ) }
</div>,
</ul>,
];
}
}
Expand Down
10 changes: 5 additions & 5 deletions blocks/library/gallery/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@
margin: -8px;
}

.blocks-gallery-image {
.blocks-gallery-item {
position: relative;

&.is-selected {
.is-selected {
outline: 4px solid $blue-medium-500;
outline-offset: -4px;
}
}

.blocks-gallery-image__inline-menu {
.blocks-gallery-item__inline-menu {
padding: 2px;
position: absolute;
top: 0;
right: 0;
background-color: $blue-medium-500;
display: inline-flex;
z-index: z-index( '.blocks-gallery-image__inline-menu' );
z-index: z-index( '.blocks-gallery-item__inline-menu' );

.components-button {
color: $white;
Expand All @@ -29,6 +29,6 @@
}
}

.blocks-gallery-image__remove {
.blocks-gallery-item__remove {
padding: 0;
}
6 changes: 3 additions & 3 deletions blocks/library/gallery/gallery-image.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class GalleryImage extends Component {

const img = url ? <img src={ url } alt={ alt } data-id={ id } /> : <Spinner />;

const className = classnames( 'blocks-gallery-image', {
const className = classnames( {
'is-selected': isSelected,
} );

Expand All @@ -45,11 +45,11 @@ class GalleryImage extends Component {
return (
<figure className={ className } onClick={ onClick }>
{ isSelected &&
<div className="blocks-gallery-image__inline-menu">
<div className="blocks-gallery-item__inline-menu">
<IconButton
icon="no-alt"
onClick={ onRemove }
className="blocks-gallery-image__remove"
className="blocks-gallery-item__remove"
label={ __( 'Remove Image' ) }
/>
</div>
Expand Down
48 changes: 42 additions & 6 deletions blocks/library/gallery/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ registerBlockType( 'core/gallery', {
type: 'array',
default: [],
source: 'query',
selector: 'div.wp-block-gallery figure.blocks-gallery-image img',
selector: 'ul.wp-block-gallery .blocks-gallery-item img',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the classname change here intentional? .blocks-gallery-item? because this should reflect what's generated in the save function (not the edit)

Copy link
Contributor

Choose a reason for hiding this comment

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

.blocks-gallery-item is intentional because it's now in <li> element, not in <figure> element. Therefore .blocks-gallery-image sounded weird for <li>.

query: {
url: {
source: 'attribute',
Expand Down Expand Up @@ -151,7 +151,7 @@ registerBlockType( 'core/gallery', {
save( { attributes } ) {
const { images, columns = defaultColumnsNumber( attributes ), align, imageCrop, linkTo } = attributes;
return (
<div className={ `align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` } >
<ul className={ `align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` } >
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't review fully today but quick note here, we should add a "deprecated" section (see quote block as an example) to avoid invalidating existing galleries.

{ images.map( ( image ) => {
let href;

Expand All @@ -167,13 +167,49 @@ registerBlockType( 'core/gallery', {
const img = <img src={ image.url } alt={ image.alt } data-id={ image.id } />;

return (
<figure key={ image.id || image.url } className="blocks-gallery-image">
{ href ? <a href={ href }>{ img }</a> : img }
</figure>
<li key={ image.id || image.url } className="blocks-gallery-item">
<figure>
{ href ? <a href={ href }>{ img }</a> : img }
</figure>
</li>
);
} ) }
</div>
</ul>
);
},

deprecated: [
Copy link
Contributor

Choose a reason for hiding this comment

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

In the deprecated section, you can just paste the old "attributes" and "save" functions before the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of them, or just the changed attributes?

{
save( { attributes } ) {
const { images, columns = defaultColumnsNumber( attributes ), align, imageCrop, linkTo } = attributes;
return (
<div className={ `align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` } >
{ images.map( ( image ) => {
let href;

switch ( linkTo ) {
case 'media':
href = image.url;
break;
case 'attachment':
href = image.link;
break;
}

const img = <img src={ image.url } alt={ image.alt } data-id={ image.id } />;

return (
<li key={ image.id || image.url } className="blocks-gallery-item">
<figure>
{ href ? <a href={ href }>{ img }</a> : img }
</figure>
</li>
);
} ) }
</div>
);
},
},
],

} );
52 changes: 19 additions & 33 deletions blocks/library/gallery/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,29 @@
.wp-block-gallery.aligncenter {
display: flex;
flex-wrap: wrap;
list-style-type: none;

.blocks-gallery-image {
.blocks-gallery-item {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep both classnames to avoid breaking old blocks styling?

margin: 8px;
display: flex;
flex-grow: 1;
flex-direction: column;
justify-content: center;

figure {
height: 100%;
margin: 0;
}

img {
display: block;
max-width: 100%;
height: auto;
}
}

// Cropped
&.is-cropped .blocks-gallery-image {
&.is-cropped .blocks-gallery-item {
img {
flex: 1;
width: 100%;
Expand All @@ -29,47 +36,26 @@
}

// Alas, IE11+ doesn't support object-fit
_:-ms-lang(x), img {
_:-ms-lang(x), figure {
height: auto;
width: auto;
}
}

&.columns-1 figure {
width: calc(100% / 1 - 16px);
}
&.columns-2 figure {
width: calc(100% / 2 - 16px);
// Responsive fallback value, 2 columns
& .blocks-gallery-item {
width: calc( 100% / 2 - 16px );
}

// Responsive fallback value, 2 columns
&.columns-3 figure,
&.columns-4 figure,
&.columns-5 figure,
&.columns-6 figure,
&.columns-7 figure,
&.columns-8 figure {
width: calc(100% / 2 - 16px);
&.columns-1 .blocks-gallery-item {
width: calc(100% / 1 - 16px);
}

@include break-small {
&.columns-3 figure {
width: calc(100% / 3 - 16px);
}
&.columns-4 figure {
width: calc(100% / 4 - 16px);
}
&.columns-5 figure {
width: calc(100% / 5 - 16px);
}
&.columns-6 figure {
width: calc(100% / 6 - 16px);
}
&.columns-7 figure {
width: calc(100% / 7 - 16px);
}
&.columns-8 figure {
width: calc(100% / 8 - 16px);
@for $i from 3 through 8 {
&.columns-#{ $i } .blocks-gallery-item {
width: calc(100% / #{ $i } - 16px );
}
}
}
}
20 changes: 12 additions & 8 deletions blocks/test/fixtures/core__gallery.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
<!-- wp:core/gallery -->
<div class="wp-block-gallery alignnone columns-2 is-cropped">
<figure class="blocks-gallery-image">
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
<figure class="blocks-gallery-image">
<img src="http://google.com/hi.png" alt="title" />
</figure>
</div>
<ul class="wp-block-gallery alignnone columns-2 is-cropped">
<li class="blocks-gallery-item">
<figure>
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
</li>
<li class="blocks-gallery-item">
<figure>
<img src="http://google.com/hi.png" alt="title" />
</figure>
</li>
</ul>
<!-- /wp:core/gallery -->
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core__gallery.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@
"imageCrop": true,
"linkTo": "none"
},
"originalContent": "<div class=\"wp-block-gallery alignnone columns-2 is-cropped\">\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t</figure>\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t</figure>\n</div>"
"originalContent": "<ul class=\"wp-block-gallery\">\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n</ul>"
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core__gallery.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"blockName": "core/gallery",
"attrs": null,
"innerBlocks": [],
"innerHTML": "\n<div class=\"wp-block-gallery alignnone columns-2 is-cropped\">\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t</figure>\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t</figure>\n</div>\n"
"innerHTML": "\n<ul class=\"wp-block-gallery\">\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n</ul>\n"
},
{
"attrs": {},
Expand Down
16 changes: 12 additions & 4 deletions blocks/test/fixtures/core__gallery.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
<!-- wp:gallery -->
<div class="wp-block-gallery alignnone columns-2 is-cropped">
<figure class="blocks-gallery-image"><img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" /></figure>
<figure class="blocks-gallery-image"><img src="http://google.com/hi.png" alt="title" /></figure>
</div>
<ul class="wp-block-gallery alignnone columns-2 is-cropped">
<li class="blocks-gallery-item">
<figure>
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
</li>
<li class="blocks-gallery-item">
<figure>
<img src="http://google.com/hi.png" alt="title" />
</figure>
</li>
</ul>
<!-- /wp:gallery -->
20 changes: 12 additions & 8 deletions blocks/test/fixtures/core__gallery__columns.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
<!-- wp:core/gallery {"columns":"1"} -->
<div class="wp-block-gallery alignnone columns-1 is-cropped">
<figure class="blocks-gallery-image">
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
<figure class="blocks-gallery-image">
<img src="http://google.com/hi.png" alt="title" />
</figure>
</div>
<ul class="wp-block-gallery columns-1 ">
<li class="blocks-gallery-item">
<figure>
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
</li>
<li class="blocks-gallery-item">
<figure>
<img src="http://google.com/hi.png" alt="title" />
</figure>
</li>
</ul>
<!-- /wp:core/gallery -->
4 changes: 2 additions & 2 deletions blocks/test/fixtures/core__gallery__columns.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"uid": "_uid_0",
"name": "core/gallery",
"isValid": true,
"isValid": false,
"attributes": {
"align": "none",
"images": [
Expand All @@ -19,6 +19,6 @@
"imageCrop": true,
"linkTo": "none"
},
"originalContent": "<div class=\"wp-block-gallery alignnone columns-1 is-cropped\">\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t</figure>\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t</figure>\n</div>"
"originalContent": "<ul class=\"wp-block-gallery columns-1 \">\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n</ul>"
}
]
2 changes: 1 addition & 1 deletion blocks/test/fixtures/core__gallery__columns.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"columns": "1"
},
"innerBlocks": [],
"innerHTML": "\n<div class=\"wp-block-gallery alignnone columns-1 is-cropped\">\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t</figure>\n\t<figure class=\"blocks-gallery-image\">\n\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t</figure>\n</div>\n"
"innerHTML": "\n<ul class=\"wp-block-gallery columns-1 \">\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"https://cldup.com/uuUqE_dXzy.jpg\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n\t<li class=\"blocks-gallery-item\">\n\t\t<figure>\n\t\t\t<img src=\"http://google.com/hi.png\" alt=\"title\" />\n\t\t</figure>\n\t</li>\n</ul>\n"
},
{
"attrs": {},
Expand Down
16 changes: 12 additions & 4 deletions blocks/test/fixtures/core__gallery__columns.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
<!-- wp:gallery {"columns":1} -->
<div class="wp-block-gallery alignnone columns-1 is-cropped">
<figure class="blocks-gallery-image"><img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" /></figure>
<figure class="blocks-gallery-image"><img src="http://google.com/hi.png" alt="title" /></figure>
</div>
<ul class="wp-block-gallery columns-1 ">
<li class="blocks-gallery-item">
<figure>
<img src="https://cldup.com/uuUqE_dXzy.jpg" alt="title" />
</figure>
</li>
<li class="blocks-gallery-item">
<figure>
<img src="http://google.com/hi.png" alt="title" />
</figure>
</li>
</ul>
<!-- /wp:gallery -->
2 changes: 1 addition & 1 deletion editor/assets/stylesheets/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $z-layers: (
'.editor-block-switcher__menu': 2,
'.components-popover__close': 2,
'.editor-block-mover': 1,
'.blocks-gallery-image__inline-menu': 20,
'.blocks-gallery-item__inline-menu': 20,
'.editor-block-settings-menu__popover': 20, // Below the header
'.editor-header': 30,
'.editor-text-editor__formatting': 30,
Expand Down
Loading