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 'insert from url' for image block #9264

Merged
merged 24 commits into from
Oct 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2ac4df9
Extract isBlobURL into blob package and update usage
talldan Aug 23, 2018
8b887c0
Add ability to insert an image in image block using a url
talldan Aug 23, 2018
99dffe3
Position url input underneath upload file buttons and apply margin
talldan Aug 23, 2018
af9c1f6
Refactor styles to avoid use of element selector
talldan Aug 23, 2018
cb269c1
Ensure form elements have bottom margin to separate them on tiny screens
talldan Aug 23, 2018
1a9c378
Change type of block used in e2e test to a separator
talldan Aug 23, 2018
e4809ae
Fix invalid reference to this.isBlobURL
talldan Aug 24, 2018
3184bb6
Add expandable section for url form on media placeholder
talldan Sep 12, 2018
c63ea9f
Change icon for media placeholder button to left/right chevron
talldan Sep 13, 2018
a8d43ca
Update snapshots
talldan Sep 13, 2018
1dced64
Try: expanding url input based on whether text is entered
talldan Sep 24, 2018
ace51f8
Try: use link popver for Insert from URL in media placeholder
talldan Sep 26, 2018
e4b3121
Implement URLPopover in MediaPlaceholder
talldan Oct 15, 2018
fe941a0
Use consistent declaration of defaults for props
talldan Oct 15, 2018
b7e90e1
Use a generalized selector for buttons in the media placeholder
talldan Oct 15, 2018
b5c0dc1
Improve copy in URL input placeholder
talldan Oct 15, 2018
4b48e5d
Make behaviour when selecting a differnent url the same as other medi…
talldan Oct 15, 2018
dd001e2
Update snapshot tests
talldan Oct 15, 2018
c824c12
Revert "Change type of block used in e2e test to a separator"
talldan Oct 15, 2018
3994517
Use onClose over onClickOutside (onClose implements onClickOutside)
talldan Oct 16, 2018
7f5cad3
Rename functions
talldan Oct 18, 2018
a433a7a
Address code review feedback
talldan Oct 18, 2018
df2947e
Update changelog for blob package
talldan Oct 18, 2018
72f302a
Updated changelog for editor package
talldan Oct 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
6 changes: 6 additions & 0 deletions packages/blob/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 2.1.0 (Unreleased)

### New Features

- Added a new `isBlobURL` function.

## 2.0.0 (2018-09-05)

### Breaking Change
Expand Down
14 changes: 14 additions & 0 deletions packages/blob/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,17 @@ export function revokeBlobURL( url ) {

delete cache[ url ];
}

/**
* Check whether a url is a blob url.
*
* @param {string} url The URL.
*
* @return {boolean} Is the url a blob url?
*/
export function isBlobURL( url ) {
Copy link
Member

Choose a reason for hiding this comment

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

We should update changelog for @wordpress/blob package and list this function as a new feature, which will require minor version increment.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@talldan talldan Oct 18, 2018

Choose a reason for hiding this comment

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

Does this look correct - aaeee84?

if ( ! url || ! url.indexOf ) {
return false;
}
return url.indexOf( 'blob:' ) === 0;
Copy link
Member

Choose a reason for hiding this comment

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

You could use lodash here if you're into that kind of thing.

export function isBlobURL( url ) {
	return startsWith( url, 'blob:' );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I avoided it there as it'd mean adding lodash as a dependency to the blob package.

Copy link
Member

Choose a reason for hiding this comment

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

}
17 changes: 17 additions & 0 deletions packages/blob/src/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {
isBlobURL,
} from '../';

describe( 'isBlobURL', () => {
it( 'returns true if the url starts with "blob:"', () => {
expect( isBlobURL( 'blob:thisbitdoesnotmatter' ) ).toBe( true );
} );

it( 'returns false if the url does not start with "blob:"', () => {
expect( isBlobURL( 'https://www.example.com' ) ).toBe( false );
} );

it( 'returns false if the url is not defined', () => {
expect( isBlobURL() ).toBe( false );
} );
} );
4 changes: 2 additions & 2 deletions packages/block-library/src/audio/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
RichText,
mediaUpload,
} from '@wordpress/editor';
import { getBlobByURL } from '@wordpress/blob';
import { getBlobByURL, isBlobURL } from '@wordpress/blob';

const ALLOWED_MEDIA_TYPES = [ 'audio' ];

Expand All @@ -40,7 +40,7 @@ class AudioEdit extends Component {
const { attributes, noticeOperations, setAttributes } = this.props;
const { id, src = '' } = attributes;

if ( ! id && src.indexOf( 'blob:' ) === 0 ) {
if ( ! id && isBlobURL( src ) ) {
const file = getBlobByURL( src );

if ( file ) {
Expand Down
27 changes: 11 additions & 16 deletions packages/block-library/src/audio/test/__snapshots__/index.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,11 @@ exports[`core/audio block edit matches snapshot 1`] = `
</span>
</div>
</div>
<form>
<input
aria-label="Audio"
class="components-placeholder__input"
placeholder="Enter URL here…"
type="url"
value=""
/>
<button
class="components-button is-button is-default is-large"
type="submit"
>
Use URL
</button>
</form>
<div
class="components-form-file-upload"
>
<button
class="components-button components-icon-button editor-media-placeholder__upload-button is-button is-default is-large"
class="components-button components-icon-button editor-media-placeholder__button is-button is-default is-large"
type="button"
>
<svg
Expand All @@ -102,6 +87,16 @@ exports[`core/audio block edit matches snapshot 1`] = `
type="file"
/>
</div>
<div
class="editor-media-placeholder__url-input-container"
>
<button
class="components-button editor-media-placeholder__button is-button is-default is-large"
type="button"
>
Insert from URL
</button>
</div>
</div>
</div>
`;
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ exports[`core/cover block edit matches snapshot 1`] = `
class="components-form-file-upload"
>
<button
class="components-button components-icon-button editor-media-placeholder__upload-button is-button is-default is-large"
class="components-button components-icon-button editor-media-placeholder__button is-button is-default is-large"
type="button"
>
<svg
Expand Down
10 changes: 3 additions & 7 deletions packages/block-library/src/file/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { getBlobByURL, revokeBlobURL } from '@wordpress/blob';
import { getBlobByURL, revokeBlobURL, isBlobURL } from '@wordpress/blob';
import {
ClipboardButton,
IconButton,
Expand Down Expand Up @@ -52,7 +52,7 @@ class FileEdit extends Component {
const { href } = attributes;

// Upload a file drag-and-dropped into the editor
if ( this.isBlobURL( href ) ) {
if ( isBlobURL( href ) ) {
const file = getBlobByURL( href );

mediaUpload( {
Expand Down Expand Up @@ -87,10 +87,6 @@ class FileEdit extends Component {
}
}

isBlobURL( url = '' ) {
return url.indexOf( 'blob:' ) === 0;
}

confirmCopyURL() {
this.setState( { showCopyConfirmation: true } );
}
Expand Down Expand Up @@ -153,7 +149,7 @@ class FileEdit extends Component {
}

const classes = classnames( className, {
'is-transient': this.isBlobURL( href ),
'is-transient': isBlobURL( href ),
} );

return (
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/gallery/gallery-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { __ } from '@wordpress/i18n';
import { BACKSPACE, DELETE } from '@wordpress/keycodes';
import { withSelect } from '@wordpress/data';
import { RichText } from '@wordpress/editor';
import { isBlobURL } from '@wordpress/blob';

class GalleryImage extends Component {
constructor() {
Expand Down Expand Up @@ -105,7 +106,7 @@ class GalleryImage extends Component {

const className = classnames( {
'is-selected': isSelected,
'is-transient': url && 0 === url.indexOf( 'blob:' ),
'is-transient': isBlobURL( url ),
Copy link
Member

Choose a reason for hiding this comment

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

Lots of nice refactoring with this new utility method 👍

} );

// Disable reason: Each block can be selected by clicking on it and we should keep the same saved markup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ exports[`core/gallery block edit matches snapshot 1`] = `
class="components-form-file-upload"
>
<button
class="components-button components-icon-button editor-media-placeholder__upload-button is-button is-default is-large"
class="components-button components-icon-button editor-media-placeholder__button is-button is-default is-large"
type="button"
>
<svg
Expand Down
103 changes: 88 additions & 15 deletions packages/block-library/src/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
*/
import { __ } from '@wordpress/i18n';
import { Component, Fragment } from '@wordpress/element';
import { getBlobByURL, revokeBlobURL } from '@wordpress/blob';
import { getBlobByURL, revokeBlobURL, isBlobURL } from '@wordpress/blob';
import {
Button,
ButtonGroup,
Expand Down Expand Up @@ -60,31 +60,56 @@ export const pickRelevantMediaFiles = ( image ) => {
return pick( image, [ 'alt', 'id', 'link', 'url', 'caption' ] );
};

/**
* Is the URL a temporary blob URL? A blob URL is one that is used temporarily
* while the image is being uploaded and will not have an id yet allocated.
*
* @param {number=} id The id of the image.
* @param {string=} url The url of the image.
*
* @return {boolean} Is the URL a Blob URL
*/
const isTemporaryImage = ( id, url ) => ! id && isBlobURL( url );

/**
* Is the url for the image hosted externally. An externally hosted image has no id
* and is not a blob url.
*
* @param {number=} id The id of the image.
* @param {string=} url The url of the image.
*
* @return {boolean} Is the url an externally hosted url?
*/
const isExternalImage = ( id, url ) => url && ! id && ! isBlobURL( url );

class ImageEdit extends Component {
constructor() {
constructor( { attributes } ) {
super( ...arguments );
this.updateAlt = this.updateAlt.bind( this );
this.updateAlignment = this.updateAlignment.bind( this );
this.onFocusCaption = this.onFocusCaption.bind( this );
this.onImageClick = this.onImageClick.bind( this );
this.onSelectImage = this.onSelectImage.bind( this );
this.onSelectURL = this.onSelectURL.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.onSetCustomHref = this.onSetCustomHref.bind( this );
this.onSetLinkDestination = this.onSetLinkDestination.bind( this );
this.toggleIsEditing = this.toggleIsEditing.bind( this );

this.state = {
captionFocused: false,
isEditing: ! attributes.url,
};
}

componentDidMount() {
const { attributes, setAttributes } = this.props;
const { id, url = '' } = attributes;

if ( ! id && url.indexOf( 'blob:' ) === 0 ) {
if ( isTemporaryImage( id, url ) ) {
const file = getBlobByURL( url );

if ( file ) {
Expand All @@ -100,10 +125,10 @@ class ImageEdit extends Component {
}

componentDidUpdate( prevProps ) {
const { id: prevID, url: prevUrl = '' } = prevProps.attributes;
const { id: prevID, url: prevURL = '' } = prevProps.attributes;
const { id, url = '' } = this.props.attributes;

if ( ! prevID && prevUrl.indexOf( 'blob:' ) === 0 && id && url.indexOf( 'blob:' ) === -1 ) {
if ( isTemporaryImage( prevID, prevURL ) && ! isTemporaryImage( id, url ) ) {
revokeBlobURL( url );
}

Expand All @@ -125,6 +150,10 @@ class ImageEdit extends Component {
return;
}

this.setState( {
isEditing: false,
} );

this.props.setAttributes( {
...pickRelevantMediaFiles( media ),
width: undefined,
Expand All @@ -151,6 +180,21 @@ class ImageEdit extends Component {
} );
}

onSelectURL( newURL ) {
const { url } = this.props.attributes;

if ( newURL !== url ) {
this.props.setAttributes( {
url: newURL,
id: undefined,
} );
}

this.setState( {
isEditing: false,
} );
}

onSetCustomHref( value ) {
this.props.setAttributes( { href: value } );
}
Expand Down Expand Up @@ -213,17 +257,33 @@ class ImageEdit extends Component {
];
}

toggleIsEditing() {
this.setState( {
isEditing: ! this.state.isEditing,
} );
}

render() {
const { isEditing } = this.state;
const { attributes, setAttributes, isLargeViewport, isSelected, className, maxWidth, noticeOperations, noticeUI, toggleSelection, isRTL } = this.props;
const { url, alt, caption, align, id, href, linkDestination, width, height } = attributes;
const isExternal = isExternalImage( id, url );

const controls = (
<BlockControls>
<BlockAlignmentToolbar
value={ align }
onChange={ this.updateAlignment }
/>
{ !! url && (
let toolbarEditButton;
if ( url ) {
if ( isExternal ) {
toolbarEditButton = (
<Toolbar>
<IconButton
className="components-icon-button components-toolbar__control"
label={ __( 'Edit image' ) }
onClick={ this.toggleIsEditing }
icon="edit"
/>
</Toolbar>
);
} else {
toolbarEditButton = (
<Toolbar>
<MediaUpload
onSelect={ this.onSelectImage }
Expand All @@ -239,11 +299,22 @@ class ImageEdit extends Component {
) }
/>
</Toolbar>
) }
);
}
}

const controls = (
<BlockControls>
<BlockAlignmentToolbar
value={ align }
onChange={ this.updateAlignment }
/>
{ toolbarEditButton }
Copy link
Member

Choose a reason for hiding this comment

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

The name toolbarEditButton would make more sense and we could shed a few lines above if we had the <Toolbar> here instead.

Suggested change
{ toolbarEditButton }
<Toolbar>{ toolbarEditButton }</Toolbar>

</BlockControls>
);

if ( ! url ) {
if ( isEditing ) {
const src = isExternal ? url : undefined;
return (
<Fragment>
{ controls }
Expand All @@ -255,17 +326,19 @@ class ImageEdit extends Component {
} }
className={ className }
onSelect={ this.onSelectImage }
onSelectURL={ this.onSelectURL }
notices={ noticeUI }
onError={ noticeOperations.createErrorNotice }
accept="image/*"
allowedTypes={ ALLOWED_MEDIA_TYPES }
value={ { id, src } }
/>
</Fragment>
);
}

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