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

Revisit image floats. #7721

Merged
merged 6 commits into from
Aug 20, 2018
Merged

Revisit image floats. #7721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c402592
Revisit image floats.
jasmussen Jul 5, 2018
a2602f3
chore: tweak documentation
tofumatt Aug 16, 2018
13a6daa
docs: Tweak some comments
tofumatt Aug 16, 2018
1362fa7
fix: Update test fixtures for new image float
tofumatt Aug 16, 2018
888fecb
Fix resize handle names
jasmussen Aug 17, 2018
35bf4c1
Revert the part that limits caption width on unresized images
jasmussen Aug 17, 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
2 changes: 1 addition & 1 deletion edit-post/assets/stylesheets/_z-index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ $z-layers: (
".editor-url-input__suggestions": 30,
".edit-post-header": 30,
".block-library-button__inline-link .editor-url-input__suggestions": 6, // URL suggestions for button block above sibling inserter
".wp-block-image__resize-handlers": 1, // Resize handlers above sibling inserter
".block-library-image__resize-handlers": 1, // Resize handlers above sibling inserter

// Side UI active buttons
".editor-block-settings-remove": 1,
Expand Down
7 changes: 4 additions & 3 deletions packages/block-library/src/image/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ class ImageEdit extends Component {
<Fragment>
{ getInspectorControls( imageWidth, imageHeight ) }
<ResizableBox
className="block-library-image__resizer"
size={
width && height ? {
width,
Expand All @@ -444,9 +445,9 @@ class ImageEdit extends Component {
maxHeight={ maxWidth / ratio }
lockAspectRatio
handleClasses={ {
right: 'wp-block-image__resize-handler-right',
bottom: 'wp-block-image__resize-handler-bottom',
left: 'wp-block-image__resize-handler-left',
right: 'block-library-image__resize-handler-right',
bottom: 'block-library-image__resize-handler-bottom',
left: 'block-library-image__resize-handler-left',
} }
enable={ {
top: false,
Expand Down
75 changes: 59 additions & 16 deletions packages/block-library/src/image/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,6 @@
position: relative;
margin: 0;

img {
display: block;
width: 100%;
}

&.is-resized img {
height: 100%;
}

&.is-transient img {
@include loading_fade;
}
Expand All @@ -20,9 +11,21 @@
}
}

.wp-block-image__resize-handler-right,
.wp-block-image__resize-handler-bottom,
.wp-block-image__resize-handler-left {
// This is necessary for the editor resize handles to accurately work on a non-floated, non-resized, small image.
.wp-block-image {
.block-library-image__resizer {
display: inline-block;

img {
display: block;
width: 100%;
}
}
}

.block-library-image__resize-handler-right,
.block-library-image__resize-handler-bottom,
.block-library-image__resize-handler-left {
display: none;
border-radius: 50%;
border: 2px solid $white;
Expand All @@ -33,22 +36,22 @@

.wp-block-image.is-focused & {
display: block;
z-index: z-index(".wp-block-image__resize-handlers");
z-index: z-index(".block-library-image__resize-handlers");
}
}

/*!rtl:begin:ignore*/
.wp-block-image__resize-handler-right {
.block-library-image__resize-handler-right {
top: calc(50% - 9px) !important;
right: -8px !important;
}

.wp-block-image__resize-handler-left {
.block-library-image__resize-handler-left {
top: calc(50% - 9px) !important;
left: -8px !important;
}

.wp-block-image__resize-handler-bottom {
.block-library-image__resize-handler-bottom {
bottom: -8px !important;
left: calc(50% - 9px) !important;
}
Expand Down Expand Up @@ -103,3 +106,43 @@
margin: -$border-width;
}
}

// Although the float markup is different in the editor compared to the frontend,
// this CSS uses the same technique to allow floats in a wide images context.
// That is, the block retains its centering and max-width, and a child inside
// is floated instead of the block itself.
[data-type="core/image"][data-align="center"],
[data-type="core/image"][data-align="left"],
[data-type="core/image"][data-align="right"] {
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like a specific fix to a generic problem, how is this solved for other floated blocks? Can't we have a generic solution to left/right aligned blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we can, though because this PR has gone on since July 5th, I wanted to limit the scope to images for now, and then expand beyond those as soon as we got this into master.

.editor-block-list__block-edit {
figure {
margin: 0;
display: table;
}

// This maps to the figcaption on the frontend.
.editor-rich-text {
display: table-caption;
caption-side: bottom;
}
}
}

[data-type="core/image"][data-align="wide"],
[data-type="core/image"][data-align="full"] {
figure img {
width: 100%;
}
}

// This is similar to above but for resized unfloated images only, where the markup is different.
[data-type="core/image"] .editor-block-list__block-edit figure.is-resized {
margin: 0;
display: table;

// This maps to the figcaption on the frontend.
.editor-rich-text {
display: table-caption;
caption-side: bottom;
}
}
49 changes: 47 additions & 2 deletions packages/block-library/src/image/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { Fragment } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import {
createBlock,
Expand Down Expand Up @@ -216,15 +217,59 @@ export const settings = {
/>
);

return (
<figure className={ classes }>
const figure = (
<Fragment>
{ href ? <a href={ href }>{ image }</a> : image }
{ caption && caption.length > 0 && <RichText.Content tagName="figcaption" value={ caption } /> }
</Fragment>
);

if ( 'left' === align || 'right' === align || 'center' === align ) {
return (
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't this meant to be an aside (from the PR description)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but per discussion in #7721 (comment), I reverted to using a div. I will update the description.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also curious about this though, because it doesn't have a className attribute yet in the fixtures the tests expect/output a <div> with a class attribute. See: 66f6f01

I'm a bit confused by that...

<figure className={ classes }>
{ figure }
</figure>
</div>
);
}

return (
<figure className={ classes }>
{ figure }
</figure>
);
},

deprecated: [
{
attributes: blockAttributes,
save( { attributes } ) {
const { url, alt, caption, align, href, width, height, id } = attributes;

const classes = classnames( {
[ `align${ align }` ]: align,
'is-resized': width || height,
} );

const image = (
<img
src={ url }
alt={ alt }
className={ id ? `wp-image-${ id }` : null }
width={ width }
height={ height }
/>
);

return (
<figure className={ classes }>
{ href ? <a href={ href }>{ image }</a> : image }
{ caption && caption.length > 0 && <RichText.Content tagName="figcaption" value={ caption } /> }
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be caption.length > 0 instead of just caption.length? 🤷‍♂️

</figure>
);
},
},
{
attributes: blockAttributes,
save( { attributes } ) {
Expand Down
41 changes: 25 additions & 16 deletions packages/block-library/src/image/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,46 @@
.wp-block-image {
width: fit-content;
max-width: 100%;

img {
max-width: 100%;
}

&.aligncenter {
display: block;
margin-left: auto;
margin-right: auto;
text-align: center;
}

&.is-resized {
width: min-content;
&.alignfull img,
&.alignwide img {
width: 100%;
}

// Emulate min-content for Edge and IE11
display: -ms-inline-grid;
-ms-grid-columns: min-content;
// Floats get an extra wrapping <aside> element, so the <figure> becomes a child.
.alignleft,
.alignright,
.aligncenter,
&.is-resized {
display: table;
margin: 0;

figcaption {
-ms-grid-row: 2;
> figcaption {
display: table-caption;
caption-side: bottom;
}
}

img {
max-width: none;
}
.alignleft {
float: left;
margin-right: 1em;
}

.alignright {
float: right;
margin-left: 1em;
}

// Supply caption styles to images, even if the theme hasn't opted in.
// Reason being: the new markup, figcaptions, are not likely to be styled in the majority of existing themes,
// so we supply the styles so as to not appear broken or unstyled in those.
// Reason being: the new markup, <figcaptions>, are not likely to be styled in the majority of existing themes,
// so we supply the styles so as to not appear broken or unstyled in those themes.
figcaption {
@include caption-style();
}
Expand Down
4 changes: 3 additions & 1 deletion packages/editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,16 @@
width: 100%;

// When images are floated, the block itself should collapse to zero height.
margin-bottom: 0;
height: 0;

// Hide block outline when an image is floated.
.editor-block-list__block-edit {
&::before {
content: none;
}

// This margin won't collapse on its own, so zero it out.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite sure what this means in this context, so maybe a bit more commenting would be handy.

margin-top: 0;
}

// Keep a 1px margin to compensate for the border/outline.
Expand Down
4 changes: 2 additions & 2 deletions phpunit/fixtures/long-content.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ <h2>A Picture is worth a Thousand Words</h2>
<!-- wp:paragraph -->
<p>Handling images and media with the utmost care is a primary focus of the new editor. Hopefully, you&#x27;ll find aspects of adding captions or going full-width with your pictures much easier and robust than before.</p>
<!-- /wp:paragraph -->
<!-- wp:image {"align":"center"} -->
<figure class="wp-block-image aligncenter"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="Beautiful landscape" /><figcaption>Give it a try. Press the &quot;wide&quot; button on the image toolbar.</figcaption></figure>
<!-- wp:image {"align":"center","className":"aligncenter"} -->
<div class="wp-block-image aligncenter"><figure class="aligncenter"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="Beautiful landscape" /><figcaption>Give it a try. Press the &quot;wide&quot; button on the image toolbar.</figcaption></figure></div>
<!-- /wp:image -->
<!-- wp:paragraph -->
<p>Try selecting and removing or editing the caption, now you don&#x27;t have to be careful about selecting the image or other text by mistake and ruining the presentation.</p>
Expand Down
10 changes: 6 additions & 4 deletions post-content.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@
<p><?php _e( 'Handling images and media with the utmost care is a primary focus of the new editor. Hopefully, you&#8217;ll find aspects of adding captions or going full-width with your pictures much easier and robust than before.', 'gutenberg' ); ?></p>
<!-- /wp:paragraph -->

<!-- wp:image {"align":"center"} -->
<figure class="wp-block-image aligncenter"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="Beautiful landscape" />
<figcaption><?php _e( 'If your theme supports it, you&#8217;ll see the "wide" button on the image toolbar. Give it a try.', 'gutenberg' ); ?></figcaption>
</figure>
<!-- wp:image {"align":"center","className":"aligncenter"} -->
<div class="wp-block-image aligncenter">
<figure class="aligncenter"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="Beautiful landscape" />
<figcaption><?php _e( 'If your theme supports it, you&#8217;ll see the "wide" button on the image toolbar. Give it a try.', 'gutenberg' ); ?></figcaption>
</figure>
</div>
<!-- /wp:image -->

<!-- wp:paragraph -->
Expand Down
2 changes: 1 addition & 1 deletion test/integration/full-content/fixtures/core__image__center-caption.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:core/image {"align":"center"} -->
<figure class="wp-block-image aligncenter"><img src="https://cldup.com/YLYhpou2oq.jpg" alt="" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>
<div class="wp-block-image aligncenter"><figure class="aligncenter"><img src="https://cldup.com/YLYhpou2oq.jpg" alt="" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>
<!-- /wp:core/image -->
5 changes: 3 additions & 2 deletions test/integration/full-content/fixtures/core__image__center-caption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
"Give it a try. Press the \"really wide\" button on the image toolbar."
],
"align": "center",
"linkDestination": "none"
"linkDestination": "none",
"className": "aligncenter"
},
"innerBlocks": [],
"originalContent": "<figure class=\"wp-block-image aligncenter\"><img src=\"https://cldup.com/YLYhpou2oq.jpg\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>"
"originalContent": "<div class=\"wp-block-image aligncenter\"><figure class=\"aligncenter\"><img src=\"https://cldup.com/YLYhpou2oq.jpg\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>"
}
]
2 changes: 1 addition & 1 deletion test/integration/full-content/fixtures/core__image__center-caption.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"align": "center"
},
"innerBlocks": [],
"innerHTML": "\n<figure class=\"wp-block-image aligncenter\"><img src=\"https://cldup.com/YLYhpou2oq.jpg\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure>\n"
"innerHTML": "\n<div class=\"wp-block-image aligncenter\"><figure class=\"aligncenter\"><img src=\"https://cldup.com/YLYhpou2oq.jpg\" alt=\"\" /><figcaption>Give it a try. Press the &quot;really wide&quot; button on the image toolbar.</figcaption></figure></div>\n"
},
{
"attrs": {},
Expand Down
4 changes: 2 additions & 2 deletions test/integration/full-content/fixtures/core__image__center-caption.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- wp:image {"align":"center"} -->
<figure class="wp-block-image aligncenter"><img src="https://cldup.com/YLYhpou2oq.jpg" alt=""/><figcaption>Give it a try. Press the "really wide" button on the image toolbar.</figcaption></figure>
<!-- wp:image {"align":"center","className":"aligncenter"} -->
<div class="wp-block-image aligncenter"><figure class="aligncenter"><img src="https://cldup.com/YLYhpou2oq.jpg" alt=""/><figcaption>Give it a try. Press the "really wide" button on the image toolbar.</figcaption></figure></div>
<!-- /wp:image -->
1 change: 0 additions & 1 deletion test/integration/full-content/fixtures/core__latest-posts.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"displayPostDate": false,
"postLayout": "list",
"columns": 3,
"align": "center",
"order": "desc",
"orderBy": "date"
},
Expand Down
1 change: 0 additions & 1 deletion test/integration/full-content/fixtures/core__latest-posts__displayPostDate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
"displayPostDate": true,
"postLayout": "list",
"columns": 3,
"align": "center",
"order": "desc",
"orderBy": "date"
},
Expand Down
2 changes: 1 addition & 1 deletion test/integration/full-content/server-registered.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"core\/block":{"attributes":{"ref":{"type":"number"}}},"core\/latest-comments":{"attributes":{"className":{"type":"string"},"commentsToShow":{"type":"number","default":5,"minimum":1,"maximum":100},"displayAvatar":{"type":"boolean","default":true},"displayDate":{"type":"boolean","default":true},"displayExcerpt":{"type":"boolean","default":true},"align":{"type":"string","enum":["center","left","right","wide","full",""]}}},"core\/archives":{"attributes":{"showPostCounts":{"type":"boolean","default":false},"displayAsDropdown":{"type":"boolean","default":false},"align":{"type":"string","default":"none"}}},"core\/latest-posts":{"attributes":{"categories":{"type":"string"},"className":{"type":"string"},"postsToShow":{"type":"number","default":5},"displayPostDate":{"type":"boolean","default":false},"postLayout":{"type":"string","default":"list"},"columns":{"type":"number","default":3},"align":{"type":"string","default":"center"},"order":{"type":"string","default":"desc"},"orderBy":{"type":"string","default":"date"}}}}
{"core\/block":{"attributes":{"ref":{"type":"number"}}},"core\/latest-comments":{"attributes":{"className":{"type":"string"},"commentsToShow":{"type":"number","default":5,"minimum":1,"maximum":100},"displayAvatar":{"type":"boolean","default":true},"displayDate":{"type":"boolean","default":true},"displayExcerpt":{"type":"boolean","default":true},"align":{"type":"string","enum":["center","left","right","wide","full",""]}}},"core\/archives":{"attributes":{"showPostCounts":{"type":"boolean","default":false},"displayAsDropdown":{"type":"boolean","default":false},"align":{"type":"string","default":"none"}}},"core\/latest-posts":{"attributes":{"categories":{"type":"string"},"className":{"type":"string"},"postsToShow":{"type":"number","default":5},"displayPostDate":{"type":"boolean","default":false},"postLayout":{"type":"string","default":"list"},"columns":{"type":"number","default":3},"align":{"type":"string"},"order":{"type":"string","default":"desc"},"orderBy":{"type":"string","default":"date"}}}}