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 ability to manually set image dimensions #5812

Merged
merged 12 commits into from
Apr 18, 2018
Merged
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
108 changes: 94 additions & 14 deletions blocks/library/image/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
import classnames from 'classnames';
import ResizableBox from 're-resizable';
import {
startCase,
find,
get,
isEmpty,
map,
get,
pick,
startCase,
} from 'lodash';

/**
Expand All @@ -18,9 +19,12 @@ import { __ } from '@wordpress/i18n';
import { Component, compose } from '@wordpress/element';
import { getBlobByURL, revokeBlobURL, viewPort } from '@wordpress/utils';
import {
Button,
ButtonGroup,
IconButton,
PanelBody,
SelectControl,
TextControl,
TextareaControl,
Toolbar,
} from '@wordpress/components';
Expand Down Expand Up @@ -55,6 +59,9 @@ class ImageBlock extends Component {
this.onSelectImage = this.onSelectImage.bind( this );
this.onSetHref = this.onSetHref.bind( this );
this.updateImageURL = this.updateImageURL.bind( this );
this.updateWidth = this.updateWidth.bind( this );
this.updateHeight = this.updateHeight.bind( this );
this.updateDimensions = this.updateDimensions.bind( this );

this.state = {
captionFocused: false,
Expand Down Expand Up @@ -98,7 +105,11 @@ class ImageBlock extends Component {
}

onSelectImage( media ) {
this.props.setAttributes( pick( media, [ 'alt', 'id', 'caption', 'url' ] ) );
this.props.setAttributes( {
...pick( media, [ 'alt', 'id', 'caption', 'url' ] ),
width: undefined,
height: undefined,
} );
}

onSetHref( value ) {
Expand Down Expand Up @@ -133,7 +144,21 @@ class ImageBlock extends Component {
}

updateImageURL( url ) {
this.props.setAttributes( { url } );
this.props.setAttributes( { url, width: undefined, height: undefined } );
}

updateWidth( width ) {
this.props.setAttributes( { width: parseInt( width, 10 ) } );
}

updateHeight( height ) {
this.props.setAttributes( { height: parseInt( height, 10 ) } );
}

updateDimensions( width = undefined, height = undefined ) {
return () => {
this.props.setAttributes( { width, height } );
};
}

getAvailableSizes() {
Expand All @@ -144,10 +169,6 @@ class ImageBlock extends Component {
const { attributes, setAttributes, isSelected, className, settings, toggleSelection } = this.props;
const { url, alt, caption, align, id, href, width, height } = attributes;

const availableSizes = this.getAvailableSizes();
const figureStyle = width ? { width } : {};
const isResizable = [ 'wide', 'full' ].indexOf( align ) === -1 && ( ! viewPort.isExtraSmall() );

const controls = (
isSelected && (
<BlockControls key="controls">
Expand Down Expand Up @@ -176,7 +197,10 @@ class ImageBlock extends Component {
)
);

if ( ! url ) {
const availableSizes = this.getAvailableSizes();
const selectedSize = find( availableSizes, ( size ) => size.source_url === url );

if ( ! url || ! selectedSize ) {
return [
controls,
<ImagePlaceholder
Expand All @@ -191,12 +215,13 @@ class ImageBlock extends Component {

const classes = classnames( className, {
'is-transient': 0 === url.indexOf( 'blob:' ),
'is-resized': !! width,
'is-resized': !! width || !! height,
'is-focused': isSelected,
} );

// Disable reason: Each block can be selected by clicking on it
const isResizable = [ 'wide', 'full' ].indexOf( align ) === -1 && ( ! viewPort.isExtraSmall() );

// Disable reason: Each block can be selected by clicking on it
/* eslint-disable jsx-a11y/no-static-element-interactions, jsx-a11y/onclick-has-role, jsx-a11y/click-events-have-key-events */
return [
controls,
Expand All @@ -211,7 +236,7 @@ class ImageBlock extends Component {
/>
{ ! isEmpty( availableSizes ) && (
<SelectControl
label={ __( 'Size' ) }
label={ __( 'Source Type' ) }
value={ url }
options={ map( availableSizes, ( size, name ) => ( {
value: size.source_url,
Expand All @@ -220,10 +245,61 @@ class ImageBlock extends Component {
onChange={ this.updateImageURL }
/>
) }
<div className="blocks-image__dimensions">
<p className="blocks-image__dimensions__row">
{ __( 'Image Dimensions' ) }
</p>
<div className="blocks-image__dimensions__row">
<TextControl
type="number"
className="blocks-image__dimensions__width"
label={ __( 'Width' ) }
value={ width !== undefined ? width : '' }
placeholder={ selectedSize.width }
Copy link
Member

Choose a reason for hiding this comment

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

This can apparently be unassigned, resulting in warnings logged to the console.

Issue at #8026.

onChange={ this.updateWidth }
/>
<TextControl
type="number"
className="blocks-image__dimensions__height"
label={ __( 'Height' ) }
value={ height !== undefined ? height : '' }
placeholder={ selectedSize.height }
onChange={ this.updateHeight }
/>
</div>
<div className="blocks-image__dimensions__row">
<ButtonGroup aria-label={ __( 'Image Size' ) }>
{ [ 25, 50, 75, 100 ].map( ( scale ) => {
const scaledWidth = Math.round( selectedSize.width * ( scale / 100 ) );
const scaledHeight = Math.round( selectedSize.height * ( scale / 100 ) );

const isCurrent = width === scaledWidth && height === scaledHeight;

return (
<Button
key={ scale }
isSmall
isPrimary={ isCurrent }
aria-pressed={ isCurrent }
onClick={ this.updateDimensions( scaledWidth, scaledHeight ) }
>
{ scale }%
</Button>
);
} ) }
</ButtonGroup>
<Button
isSmall
onClick={ this.updateDimensions() }
>
{ __( 'Reset' ) }
</Button>
</div>
</div>
</PanelBody>
</InspectorControls>
),
<figure key="image" className={ classes } style={ figureStyle }>
<figure key="image" className={ classes }>
<ImageSize src={ url } dirtynessTrigger={ align }>
{ ( sizes ) => {
const {
Expand All @@ -239,7 +315,11 @@ class ImageBlock extends Component {
const img = <img src={ url } alt={ alt } onClick={ this.onImageClick } />;

if ( ! isResizable || ! imageWidthWithinContainer ) {
return img;
return (
<div style={ { width, height } }>
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes feels a bit weird, we're adding an unnecessary extra div for me and I believe this div is not present in the "save" representation? Can we avoid this extra div? Why can't we add these styles to the "figure"?

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 changes feels a bit weird, we're adding an unnecessary extra div for me and I believe this div is not present in the "save" representation?

Our CSS rules (namely .wp-block-image.is-resized img { height: 100% }) rely on the img being contained in something that has an explicit width and height. On desktop, this is true because re-resizable wraps the image in a div with an inline width and height. On mobile, however, we don't load re-resizable and so our CSS rules don't work.

By wrapping the img with our own div on mobile, our CSS rules work on both mobile and desktop because our markup has a similar structure.

Why can't we add these styles to the "figure"?

We can't set a height on the figure because we do not know how tall the caption underneath the image is.

{ img }
</div>
);
}

const currentWidth = width || imageWidthWithinContainer;
Expand Down
31 changes: 31 additions & 0 deletions blocks/library/image/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
width: 100%;
}

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

&.is-transient img {
@include loading_fade;
}
Expand Down Expand Up @@ -92,3 +96,30 @@
margin-right: auto;
}
}

.edit-post-sidebar .blocks-image__dimensions {
margin-bottom: 1em;

.blocks-image__dimensions__row {
display: flex;
justify-content: space-between;

.blocks-image__dimensions__width,
.blocks-image__dimensions__height {
margin-bottom: 0.5em;

// Fix the text and placeholder text being misaligned in Safari
input {
line-height: 1.25;
}
}

.blocks-image__dimensions__width {
margin-right: 5px;
}

.blocks-image__dimensions__height {
margin-left: 5px;
}
}
}
1 change: 1 addition & 0 deletions blocks/library/paragraph/editor.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.blocks-font-size__main {
display: flex;
justify-content: space-between;
margin-bottom: 1em;
}

.blocks-paragraph__custom-size-slider {
Expand Down
1 change: 0 additions & 1 deletion components/button-group/style.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.components-button-group {
display: inline-block;
margin-bottom: 20px;

.components-button {
border-radius: 0;
Expand Down
2 changes: 1 addition & 1 deletion components/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Button extends Component {
...additionalProps
} = this.props;
const classes = classnames( 'components-button', className, {
button: ( isPrimary || isLarge ),
button: ( isPrimary || isLarge || isSmall ),
'button-primary': isPrimary,
'button-large': isLarge,
'button-small': isSmall,
Expand Down
2 changes: 1 addition & 1 deletion components/button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe( 'Button', () => {

it( 'should render a button element with button-small class', () => {
const button = shallow( <Button isSmall /> );
expect( button.hasClass( 'button' ) ).toBe( false );
expect( button.hasClass( 'button' ) ).toBe( true );
expect( button.hasClass( 'button-large' ) ).toBe( false );
expect( button.hasClass( 'button-small' ) ).toBe( true );
expect( button.hasClass( 'button-primary' ) ).toBe( false );
Expand Down
10 changes: 9 additions & 1 deletion components/text-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,19 @@ Type of the input element to render. Defaults to "text".

### value

The current value of the input
The current value of the input.

- Type: `Number`
- Required: Yes

### className

The class that will be added with "components-base-control" to the classes of the wrapper div.
If no className is passed only components-base-control is used.

- Type: `String`
- Required: No

### onChange

A function that receives the value of the input.
Expand Down
4 changes: 2 additions & 2 deletions components/text-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import BaseControl from '../base-control';
import withInstanceId from '../higher-order/with-instance-id';
import './style.scss';

function TextControl( { label, value, help, instanceId, onChange, type = 'text', ...props } ) {
function TextControl( { label, value, help, className, instanceId, onChange, type = 'text', ...props } ) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch we should have added this prop since the start 👍 We should also add the className prop to the component readme.

Copy link
Member Author

Choose a reason for hiding this comment

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

👌 cfd6d55

const id = `inspector-text-control-${ instanceId }`;
const onChangeValue = ( event ) => onChange( event.target.value );

return (
<BaseControl label={ label } id={ id } help={ help }>
<BaseControl label={ label } id={ id } help={ help } className={ className }>
<input className="components-text-control__input"
type={ type }
id={ id }
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"mousetrap": "1.6.1",
"prop-types": "15.5.10",
"querystringify": "1.0.0",
"re-resizable": "4.0.3",
"re-resizable": "4.4.8",
"react": "16.3.0",
"react-autosize-textarea": "3.0.2",
"react-click-outside": "2.3.1",
Expand Down