From 2ac4df9dadac18278112abe53ca1e7ef963587a8 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 15:18:46 +0800 Subject: [PATCH 01/24] Extract isBlobURL into blob package and update usage Co-authored-by: imath --- packages/blob/src/index.js | 14 +++++++++++++ packages/blob/src/test/index.js | 17 +++++++++++++++ packages/block-library/src/audio/edit.js | 4 ++-- packages/block-library/src/file/edit.js | 8 ++----- .../src/gallery/gallery-image.js | 3 ++- packages/block-library/src/image/edit.js | 21 ++++++++++++++----- packages/block-library/src/video/edit.js | 4 ++-- 7 files changed, 55 insertions(+), 16 deletions(-) create mode 100644 packages/blob/src/test/index.js diff --git a/packages/blob/src/index.js b/packages/blob/src/index.js index 6d8c1c2fd96b0..5b39dfdaf7c4a 100644 --- a/packages/blob/src/index.js +++ b/packages/blob/src/index.js @@ -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 ) { + if ( ! url || ! url.indexOf ) { + return false; + } + return url.indexOf( 'blob:' ) === 0; +} diff --git a/packages/blob/src/test/index.js b/packages/blob/src/test/index.js new file mode 100644 index 0000000000000..49adedc33e58c --- /dev/null +++ b/packages/blob/src/test/index.js @@ -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 ); + } ); +} ); diff --git a/packages/block-library/src/audio/edit.js b/packages/block-library/src/audio/edit.js index 9231e2cb1a28b..76ed3382f7009 100644 --- a/packages/block-library/src/audio/edit.js +++ b/packages/block-library/src/audio/edit.js @@ -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' ]; @@ -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 ) { diff --git a/packages/block-library/src/file/edit.js b/packages/block-library/src/file/edit.js index 132f55bd6fc48..5e20c67419dc3 100644 --- a/packages/block-library/src/file/edit.js +++ b/packages/block-library/src/file/edit.js @@ -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, @@ -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( { @@ -87,10 +87,6 @@ class FileEdit extends Component { } } - isBlobURL( url = '' ) { - return url.indexOf( 'blob:' ) === 0; - } - confirmCopyURL() { this.setState( { showCopyConfirmation: true } ); } diff --git a/packages/block-library/src/gallery/gallery-image.js b/packages/block-library/src/gallery/gallery-image.js index aeb4fd5b2101e..83ba5dc8dd9d7 100644 --- a/packages/block-library/src/gallery/gallery-image.js +++ b/packages/block-library/src/gallery/gallery-image.js @@ -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() { @@ -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 ), } ); // Disable reason: Each block can be selected by clicking on it and we should keep the same saved markup diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 2051ae580be1d..4ae88996a4c44 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -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, @@ -60,6 +60,17 @@ 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 isTemporaryBlobURL = ( id, url ) => ! id && isBlobURL( url ); + class ImageEdit extends Component { constructor() { super( ...arguments ); @@ -84,7 +95,7 @@ class ImageEdit extends Component { const { attributes, setAttributes } = this.props; const { id, url = '' } = attributes; - if ( ! id && url.indexOf( 'blob:' ) === 0 ) { + if ( isTemporaryBlobURL( id, url ) ) { const file = getBlobByURL( url ); if ( file ) { @@ -100,10 +111,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 ( isTemporaryBlobURL( prevID, prevURL ) && ! isTemporaryBlobURL( id, url ) ) { revokeBlobURL( url ); } @@ -265,7 +276,7 @@ class ImageEdit extends Component { } const classes = classnames( className, { - 'is-transient': 0 === url.indexOf( 'blob:' ), + 'is-transient': isBlobURL( url ), 'is-resized': !! width || !! height, 'is-focused': isSelected, } ); diff --git a/packages/block-library/src/video/edit.js b/packages/block-library/src/video/edit.js index 7158496b1a4e2..1d9b7b195a2eb 100644 --- a/packages/block-library/src/video/edit.js +++ b/packages/block-library/src/video/edit.js @@ -22,7 +22,7 @@ import { RichText, mediaUpload, } from '@wordpress/editor'; -import { getBlobByURL } from '@wordpress/blob'; +import { getBlobByURL, isBlobURL } from '@wordpress/blob'; const ALLOWED_MEDIA_TYPES = [ 'video' ]; @@ -45,7 +45,7 @@ class VideoEdit extends Component { componentDidMount() { 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 ) { mediaUpload( { From 8b887c05f0a8f0ebe8a09d5a0dc7c94bc0682ecf Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 15:25:34 +0800 Subject: [PATCH 02/24] Add ability to insert an image in image block using a url Co-authored-by: imath --- packages/block-library/src/image/edit.js | 90 +++++++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 4ae88996a4c44..01b9cea087347 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -71,6 +71,17 @@ export const pickRelevantMediaFiles = ( image ) => { */ const isTemporaryBlobURL = ( 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 isExternalURL = ( id, url ) => url && ! id && ! isBlobURL( url ); + class ImageEdit extends Component { constructor() { super( ...arguments ); @@ -79,15 +90,18 @@ class ImageEdit extends Component { 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: ! this.props.attributes.url, }; } @@ -136,6 +150,10 @@ class ImageEdit extends Component { return; } + this.setState( { + isEditing: false, + } ); + this.props.setAttributes( { ...pickRelevantMediaFiles( media ), width: undefined, @@ -162,6 +180,29 @@ class ImageEdit extends Component { } ); } + onSelectURL( newURL ) { + const { url, id } = this.props.attributes; + + this.setState( { + isEditing: false, + } ); + + if ( newURL !== url && id ) { + // User is switching from an uploaded image an external image. + // Unset any properties that relate to an uploaded image. + this.props.setAttributes( { + url: newURL, + alt: undefined, + id: undefined, + caption: undefined, + } ); + } else { + this.props.setAttributes( { + url: newURL, + } ); + } + } + onSetCustomHref( value ) { this.props.setAttributes( { href: value } ); } @@ -224,17 +265,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 = isExternalURL( id, url ); - const controls = ( - - - { !! url && ( + let toolbarEditButton; + if ( url ) { + if ( isExternal ) { + toolbarEditButton = ( + + + + ); + } else { + toolbarEditButton = ( - ) } + ); + } + } + + const controls = ( + + + { toolbarEditButton } ); - if ( ! url ) { + if ( isEditing ) { + const src = isExternal ? url : undefined; return ( { controls } @@ -266,10 +334,12 @@ 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 } } /> ); @@ -399,7 +469,7 @@ class ImageEdit extends Component { // Disable reason: Image itself is not meant to be // interactive, but should direct focus to block // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - const img = {; + const img = ( { ); if ( ! isResizable || ! imageWidthWithinContainer ) { return ( From 99dffe348e1b0f4d16e91b6af99ac8b9962ccfac Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 15:45:51 +0800 Subject: [PATCH 03/24] Position url input underneath upload file buttons and apply margin Co-authored-by: imath --- .../audio/test/__snapshots__/index.js.snap | 32 ++++++++-------- .../video/test/__snapshots__/index.js.snap | 32 ++++++++-------- .../src/components/media-placeholder/index.js | 37 ++++++++++--------- .../components/media-placeholder/style.scss | 8 +--- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/packages/block-library/src/audio/test/__snapshots__/index.js.snap b/packages/block-library/src/audio/test/__snapshots__/index.js.snap index 71c38018c590f..a339f37c2fd61 100644 --- a/packages/block-library/src/audio/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/audio/test/__snapshots__/index.js.snap @@ -58,21 +58,6 @@ exports[`core/audio block edit matches snapshot 1`] = ` -
- - -
@@ -102,6 +87,23 @@ exports[`core/audio block edit matches snapshot 1`] = ` type="file" />
+
+ + +
`; diff --git a/packages/block-library/src/video/test/__snapshots__/index.js.snap b/packages/block-library/src/video/test/__snapshots__/index.js.snap index 33d0d470ec7da..011754f8797bd 100644 --- a/packages/block-library/src/video/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/video/test/__snapshots__/index.js.snap @@ -58,21 +58,6 @@ exports[`core/video block edit matches snapshot 1`] = ` -
- - -
@@ -102,6 +87,23 @@ exports[`core/video block edit matches snapshot 1`] = ` type="file" />
+
+ + +
`; diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index bb2dc71b3d797..74445196ce67e 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -128,23 +128,6 @@ class MediaPlaceholder extends Component { onFilesDrop={ this.onFilesUpload } onHTMLDrop={ onHTMLDrop } /> - { onSelectURL && ( -
- - -
- ) } ) } /> + { onSelectURL && ( +
+ + +
+ ) } ); } diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index 43a862d045c34..b75b3d11537bb 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -1,10 +1,6 @@ .editor-media-placeholder { - .components-placeholder__input { - margin-top: 0.5em; - } - - .components-button.is-large { - margin-top: 0.5em; + &__url-input-form { + margin-top: 1em; } .components-placeholder__fieldset { From af9c1f673cdaf28f653f64fa8c77b8dcc50b636a Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 15:58:06 +0800 Subject: [PATCH 04/24] Refactor styles to avoid use of element selector Co-authored-by: imath --- .../src/components/media-placeholder/style.scss | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index b75b3d11537bb..368fd30ea946b 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -1,14 +1,9 @@ -.editor-media-placeholder { - &__url-input-form { - margin-top: 1em; - } +.editor-media-placeholder .components-placeholder__fieldset { + max-width: 400px; - .components-placeholder__fieldset { - max-width: 400px; - - form { - max-width: none; - } + .editor-media-placeholder__url-input-form { + margin-top: 1em; + max-width: none; } } From cb269c13d8ba205f3d210baddb3bcca8438a7e2c Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 23 Aug 2018 16:06:14 +0800 Subject: [PATCH 05/24] Ensure form elements have bottom margin to separate them on tiny screens Co-authored-by: imath --- packages/editor/src/components/media-placeholder/index.js | 2 +- .../editor/src/components/media-placeholder/style.scss | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 74445196ce67e..8ecbe4bb50304 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -156,7 +156,7 @@ class MediaPlaceholder extends Component { > Date: Thu, 23 Aug 2018 16:40:04 +0800 Subject: [PATCH 06/24] Change type of block used in e2e test to a separator As mentioned in the comment above, this test will not work for blocks that contain an input. The image block now contains a url input field causing the test to fail. Co-authored-by: imath --- test/e2e/specs/__snapshots__/adding-blocks.test.js.snap | 6 +++--- test/e2e/specs/adding-blocks.test.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap b/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap index 30b83349a7030..fec0880fed697 100644 --- a/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap +++ b/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap @@ -9,9 +9,9 @@ exports[`adding blocks Should insert content using the placeholder and the regul

Quote block

- -
\\"\\"/
- + +
+

diff --git a/test/e2e/specs/adding-blocks.test.js b/test/e2e/specs/adding-blocks.test.js index f836c89418749..6ac3ae211d9d4 100644 --- a/test/e2e/specs/adding-blocks.test.js +++ b/test/e2e/specs/adding-blocks.test.js @@ -30,10 +30,10 @@ describe( 'adding blocks', () => { await page.keyboard.press( 'ArrowDown' ); // Focus should be moved to block focus boundary on a block which does - // not have its own inputs (e.g. image). Proceeding to press enter will + // not have its own inputs (e.g. separator). Proceeding to press enter will // append the default block. Pressing backspace on the focused block // will remove it. - await page.keyboard.type( '/image' ); + await page.keyboard.type( '/separator' ); await page.keyboard.press( 'Enter' ); await page.keyboard.press( 'Enter' ); expect( await getEditedPostContent() ).toMatchSnapshot(); From e4809ae50b92e1afee50a23a4f48b4e306ca64f3 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Fri, 24 Aug 2018 17:11:22 +0800 Subject: [PATCH 07/24] Fix invalid reference to this.isBlobURL --- packages/block-library/src/file/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/file/edit.js b/packages/block-library/src/file/edit.js index 5e20c67419dc3..0a5161475bae4 100644 --- a/packages/block-library/src/file/edit.js +++ b/packages/block-library/src/file/edit.js @@ -149,7 +149,7 @@ class FileEdit extends Component { } const classes = classnames( className, { - 'is-transient': this.isBlobURL( href ), + 'is-transient': isBlobURL( href ), } ); return ( From 3184bb6cf2ecf6b031a8138907c63190c8ece678 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 12 Sep 2018 18:04:39 +0100 Subject: [PATCH 08/24] Add expandable section for url form on media placeholder --- .../src/components/media-placeholder/index.js | 61 +++++++++++++------ .../components/media-placeholder/style.scss | 18 ++++-- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 8ecbe4bb50304..fbd9c821e2da3 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -9,6 +9,7 @@ import classnames from 'classnames'; */ import { Button, + IconButton, FormFileUpload, Placeholder, DropZone, @@ -28,11 +29,13 @@ class MediaPlaceholder extends Component { super( ...arguments ); this.state = { src: '', + isURLInputExpanded: false, }; this.onChangeSrc = this.onChangeSrc.bind( this ); this.onSubmitSrc = this.onSubmitSrc.bind( this ); this.onUpload = this.onUpload.bind( this ); this.onFilesUpload = this.onFilesUpload.bind( this ); + this.toggleURLInputVisibility = this.toggleURLInputVisibility.bind( this ); } getAllowedTypes() { @@ -101,6 +104,10 @@ class MediaPlaceholder extends Component { } ); } + toggleURLInputVisibility() { + this.setState( { isURLInputExpanded: ! this.state.isURLInputExpanded } ); + } + render() { const { accept, @@ -114,7 +121,13 @@ class MediaPlaceholder extends Component { multiple = false, notices, } = this.props; + + const { + isURLInputExpanded, + } = this.state; + const allowedTypes = this.getAllowedTypes(); + return ( { onSelectURL && ( -
- - -
+ className="editor-media-placeholder__url-input-expandable-button" + icon={ isURLInputExpanded ? 'arrow-up' : 'arrow-down' } + onClick={ this.toggleURLInputVisibility } + isToggled={ isURLInputExpanded } + aria-expanded={ isURLInputExpanded } + > + { ! isURLInputExpanded ? __( 'Enter URL' ) : null } + + { isURLInputExpanded && +
+ + +
+ } + ) } ); diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index f7b7f31ffa788..07c077289df38 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -2,15 +2,10 @@ max-width: 400px; .editor-media-placeholder__url-input-form { - margin-top: 0.5rem; max-width: none; } } -.editor-media-placeholder__url-input { - margin-bottom: 0.5rem; -} - .editor-media-placeholder__upload-button.components-button { margin-right: 5px; margin-bottom: 0.5rem; @@ -24,3 +19,16 @@ color: $dark-gray-800; } } + +.editor-media-placeholder__url-input-expandable-section { + width: 100%; + min-height: 32px; + display: flex; + justify-content: center; + align-items: center; +} + +.editor-media-placeholder__url-input-expandable-button.is-large { + margin-right: 5px; + padding-left: 6px; +} From c63ea9f7bc0be8201469467a99f3b8e8fa68adb2 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 13 Sep 2018 11:31:16 +0100 Subject: [PATCH 09/24] Change icon for media placeholder button to left/right chevron --- packages/editor/src/components/media-placeholder/index.js | 2 +- packages/editor/src/components/media-placeholder/style.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index fbd9c821e2da3..245c02f0d3527 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -167,7 +167,7 @@ class MediaPlaceholder extends Component { Date: Thu, 13 Sep 2018 11:31:30 +0100 Subject: [PATCH 10/24] Update snapshots --- .../audio/test/__snapshots__/index.js.snap | 34 ++++++++++++------- .../video/test/__snapshots__/index.js.snap | 34 ++++++++++++------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/packages/block-library/src/audio/test/__snapshots__/index.js.snap b/packages/block-library/src/audio/test/__snapshots__/index.js.snap index a339f37c2fd61..1d1fd8badb1ba 100644 --- a/packages/block-library/src/audio/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/audio/test/__snapshots__/index.js.snap @@ -87,23 +87,31 @@ exports[`core/audio block edit matches snapshot 1`] = ` type="file" /> -
- -
+ `; diff --git a/packages/block-library/src/video/test/__snapshots__/index.js.snap b/packages/block-library/src/video/test/__snapshots__/index.js.snap index 011754f8797bd..c110604aab4ec 100644 --- a/packages/block-library/src/video/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/video/test/__snapshots__/index.js.snap @@ -87,23 +87,31 @@ exports[`core/video block edit matches snapshot 1`] = ` type="file" /> -
- -
+ `; From 1dced644b369a820c83aa0c499d27ec4ed5df568 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 24 Sep 2018 13:22:34 +0100 Subject: [PATCH 11/24] Try: expanding url input based on whether text is entered --- .../src/components/media-placeholder/index.js | 67 +++++++++---------- .../components/media-placeholder/style.scss | 29 ++++---- 2 files changed, 46 insertions(+), 50 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 245c02f0d3527..f8e01dcb621d5 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -9,7 +9,6 @@ import classnames from 'classnames'; */ import { Button, - IconButton, FormFileUpload, Placeholder, DropZone, @@ -76,9 +75,14 @@ class MediaPlaceholder extends Component { } onChangeSrc( event ) { - this.setState( { - src: event.target.value, - } ); + const value = event.target.value; + + const stateUpdate = { + src: value, + isURLInputExpanded: value.length > 0, + }; + + this.setState( stateUpdate ); } onSubmitSrc( event ) { @@ -105,7 +109,7 @@ class MediaPlaceholder extends Component { } toggleURLInputVisibility() { - this.setState( { isURLInputExpanded: ! this.state.isURLInputExpanded } ); + this.setState( { isURLInputExpanded: true } ); } render() { @@ -126,6 +130,10 @@ class MediaPlaceholder extends Component { isURLInputExpanded, } = this.state; + const urlInputFormClasses = classnames( 'editor-media-placeholder__url-input-form', { + 'is-expanded': isURLInputExpanded, + } ); + const allowedTypes = this.getAllowedTypes(); return ( @@ -163,38 +171,25 @@ class MediaPlaceholder extends Component { ) } /> { onSelectURL && ( -
- + + - - } -
+ type="submit"> + { __( 'Use URL' ) } + + ) } ); diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index 6df578a5e3654..244c9767be55a 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -2,7 +2,21 @@ max-width: 400px; .editor-media-placeholder__url-input-form { - max-width: none; + max-width: 210px; + height: 32px; + margin: -1px 0 0 6px; + + .editor-media-placeholder__url-input-submit-button { + display: none; + } + + &.is-expanded { + max-width: none; + + .editor-media-placeholder__url-input-submit-button { + display: block; + } + } } } @@ -19,16 +33,3 @@ color: $dark-gray-800; } } - -.editor-media-placeholder__url-input-expandable-section { - width: 100%; - min-height: 32px; - display: flex; - justify-content: center; - align-items: center; -} - -.editor-media-placeholder__url-input-expandable-button.is-large { - margin-right: 5px; - padding-left: 10px; -} From ace51f85281a6ff302e541fd85487f20dfe08c1e Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Wed, 26 Sep 2018 13:01:09 +0100 Subject: [PATCH 12/24] Try: use link popver for Insert from URL in media placeholder --- .../src/components/media-placeholder/index.js | 95 ++++++++++++------- .../components/media-placeholder/style.scss | 22 +---- 2 files changed, 65 insertions(+), 52 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index f8e01dcb621d5..e1c671ecd2a19 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -12,6 +12,8 @@ import { FormFileUpload, Placeholder, DropZone, + Popover, + IconButton, } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { Component } from '@wordpress/element'; @@ -23,18 +25,51 @@ import deprecated from '@wordpress/deprecated'; import MediaUpload from '../media-upload'; import { mediaUpload } from '../../utils/'; +const stopPropagation = ( event ) => event.stopPropagation(); + +const UrlInputPopover = ( { value, onChange, onSubmit, onClickOutside } ) => ( + + { // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar + /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ } +
+
+
+ +
+ +
+
+ { /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ } + +); + class MediaPlaceholder extends Component { constructor() { super( ...arguments ); this.state = { src: '', - isURLInputExpanded: false, + isURLInputVisible: false, }; this.onChangeSrc = this.onChangeSrc.bind( this ); this.onSubmitSrc = this.onSubmitSrc.bind( this ); this.onUpload = this.onUpload.bind( this ); this.onFilesUpload = this.onFilesUpload.bind( this ); - this.toggleURLInputVisibility = this.toggleURLInputVisibility.bind( this ); + this.openURLInput = this.openURLInput.bind( this ); + this.closeURLInput = this.closeURLInput.bind( this ); } getAllowedTypes() { @@ -75,20 +110,14 @@ class MediaPlaceholder extends Component { } onChangeSrc( event ) { - const value = event.target.value; - - const stateUpdate = { - src: value, - isURLInputExpanded: value.length > 0, - }; - - this.setState( stateUpdate ); + this.setState( { src: event.target.value } ); } onSubmitSrc( event ) { event.preventDefault(); if ( this.state.src && this.props.onSelectURL ) { this.props.onSelectURL( this.state.src ); + this.closeURLInput(); } } @@ -108,8 +137,12 @@ class MediaPlaceholder extends Component { } ); } - toggleURLInputVisibility() { - this.setState( { isURLInputExpanded: true } ); + openURLInput() { + this.setState( { isURLInputVisible: true } ); + } + + closeURLInput() { + this.setState( { isURLInputVisible: false } ); } render() { @@ -127,12 +160,10 @@ class MediaPlaceholder extends Component { } = this.props; const { - isURLInputExpanded, + isURLInputVisible, } = this.state; - const urlInputFormClasses = classnames( 'editor-media-placeholder__url-input-form', { - 'is-expanded': isURLInputExpanded, - } ); + const toggleURLInput = ! isURLInputVisible ? this.openURLInput : undefined; const allowedTypes = this.getAllowedTypes(); @@ -171,25 +202,25 @@ class MediaPlaceholder extends Component { ) } /> { onSelectURL && ( -
- -
+ { isURLInputVisible && ( + + ) } + ) } ); diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index 244c9767be55a..96f429d3d49fa 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -1,23 +1,5 @@ -.editor-media-placeholder .components-placeholder__fieldset { - max-width: 400px; - - .editor-media-placeholder__url-input-form { - max-width: 210px; - height: 32px; - margin: -1px 0 0 6px; - - .editor-media-placeholder__url-input-submit-button { - display: none; - } - - &.is-expanded { - max-width: none; - - .editor-media-placeholder__url-input-submit-button { - display: block; - } - } - } +.editor-media-placeholder__url-input-container { + width: 100%; } .editor-media-placeholder__upload-button.components-button { From e4b3121c3e69a556defd74b3db5952d3f30f0656 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 15 Oct 2018 12:23:29 +0800 Subject: [PATCH 13/24] Implement URLPopover in MediaPlaceholder --- .../src/components/media-placeholder/index.js | 61 +++++++------------ .../components/media-placeholder/style.scss | 22 +++++++ 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index e1c671ecd2a19..8ec5d5ff71831 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -12,7 +12,6 @@ import { FormFileUpload, Placeholder, DropZone, - Popover, IconButton, } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; @@ -23,40 +22,9 @@ import deprecated from '@wordpress/deprecated'; * Internal dependencies */ import MediaUpload from '../media-upload'; +import URLPopover from '../url-popover'; import { mediaUpload } from '../../utils/'; -const stopPropagation = ( event ) => event.stopPropagation(); - -const UrlInputPopover = ( { value, onChange, onSubmit, onClickOutside } ) => ( - - { // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar - /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ } -
-
-
- -
- -
-
- { /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ } - -); - class MediaPlaceholder extends Component { constructor() { super( ...arguments ); @@ -213,12 +181,29 @@ class MediaPlaceholder extends Component { { __( 'Insert from URL' ) } { isURLInputVisible && ( - + > +
+ + + + ) } ) } diff --git a/packages/editor/src/components/media-placeholder/style.scss b/packages/editor/src/components/media-placeholder/style.scss index 96f429d3d49fa..b7a4fe25e407b 100644 --- a/packages/editor/src/components/media-placeholder/style.scss +++ b/packages/editor/src/components/media-placeholder/style.scss @@ -2,6 +2,28 @@ width: 100%; } +.editor-media-placeholder__url-input-form { + display: flex; + + // Selector requires a lot of specificity to override base styles. + input[type="url"].editor-media-placeholder__url-input-field { + width: 100%; + @include break-small() { + width: 300px; + } + + flex-grow: 1; + border: none; + border-radius: 0; + margin: 2px; + + } +} + +.editor-media-placeholder__url-input-submit-button { + flex-shrink: 1; +} + .editor-media-placeholder__upload-button.components-button { margin-right: 5px; margin-bottom: 0.5rem; From fe941a0a8a58a181e8988787e11730d6a48e8601 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 15 Oct 2018 12:25:00 +0800 Subject: [PATCH 14/24] Use consistent declaration of defaults for props --- packages/editor/src/components/url-popover/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/url-popover/index.js b/packages/editor/src/components/url-popover/index.js index cf7a8b3e7787f..1045e33b132cc 100644 --- a/packages/editor/src/components/url-popover/index.js +++ b/packages/editor/src/components/url-popover/index.js @@ -29,8 +29,8 @@ class URLPopover extends Component { const { children, renderSettings, - position, onClickOutside, + position = 'bottom center', focusOnMount = 'firstElement', } = this.props; @@ -44,7 +44,7 @@ class URLPopover extends Component {
From b7e90e1ab4181355a8ff6a73c49e7df7d8418d11 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 15 Oct 2018 12:37:38 +0800 Subject: [PATCH 15/24] Use a generalized selector for buttons in the media placeholder --- .../editor/src/components/media-placeholder/index.js | 10 +++++++--- .../editor/src/components/media-placeholder/style.scss | 7 ++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index 8ec5d5ff71831..806931f12b4a3 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -150,7 +150,7 @@ class MediaPlaceholder extends Component { /> ( - ) } @@ -174,7 +178,7 @@ class MediaPlaceholder extends Component { className="editor-media-placeholder__url-input-container" >
diff --git a/packages/block-library/src/cover/test/__snapshots__/index.js.snap b/packages/block-library/src/cover/test/__snapshots__/index.js.snap index d06c221b6e4ea..0592141c6088c 100644 --- a/packages/block-library/src/cover/test/__snapshots__/index.js.snap +++ b/packages/block-library/src/cover/test/__snapshots__/index.js.snap @@ -62,7 +62,7 @@ exports[`core/cover block edit matches snapshot 1`] = ` class="components-form-file-upload" > From c824c12002e82c91c70506ef0d48a9464ba1da68 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Mon, 15 Oct 2018 13:15:29 +0800 Subject: [PATCH 19/24] Revert "Change type of block used in e2e test to a separator" This reverts commit 6c8546c29070827d62d37b1f0198ad302f79ec49. --- test/e2e/specs/__snapshots__/adding-blocks.test.js.snap | 6 +++--- test/e2e/specs/adding-blocks.test.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap b/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap index fec0880fed697..30b83349a7030 100644 --- a/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap +++ b/test/e2e/specs/__snapshots__/adding-blocks.test.js.snap @@ -9,9 +9,9 @@ exports[`adding blocks Should insert content using the placeholder and the regul

Quote block

- -
- + +
\\"\\"/
+

diff --git a/test/e2e/specs/adding-blocks.test.js b/test/e2e/specs/adding-blocks.test.js index 6ac3ae211d9d4..f836c89418749 100644 --- a/test/e2e/specs/adding-blocks.test.js +++ b/test/e2e/specs/adding-blocks.test.js @@ -30,10 +30,10 @@ describe( 'adding blocks', () => { await page.keyboard.press( 'ArrowDown' ); // Focus should be moved to block focus boundary on a block which does - // not have its own inputs (e.g. separator). Proceeding to press enter will + // not have its own inputs (e.g. image). Proceeding to press enter will // append the default block. Pressing backspace on the focused block // will remove it. - await page.keyboard.type( '/separator' ); + await page.keyboard.type( '/image' ); await page.keyboard.press( 'Enter' ); await page.keyboard.press( 'Enter' ); expect( await getEditedPostContent() ).toMatchSnapshot(); From 39945179c7ca55446eb9318a9d03ba2e2bf257e7 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Tue, 16 Oct 2018 16:23:31 +0800 Subject: [PATCH 20/24] Use onClose over onClickOutside (onClose implements onClickOutside) --- packages/editor/src/components/media-placeholder/index.js | 4 +--- packages/editor/src/components/url-popover/README.md | 7 ++++--- packages/editor/src/components/url-popover/index.js | 4 +++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index bb94755464431..ad8d79a4fc12d 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -185,9 +185,7 @@ class MediaPlaceholder extends Component { { __( 'Insert from URL' ) } { isURLInputVisible && ( - +
Edit URL { isVisible && ( (
From 7f5cad3daf444b9798e4c113e9058d7a07440ba6 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 18 Oct 2018 16:30:57 +0800 Subject: [PATCH 21/24] Rename functions --- packages/block-library/src/image/edit.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 748a790e62f33..5471dcb0a82cc 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -69,7 +69,7 @@ export const pickRelevantMediaFiles = ( image ) => { * * @return {boolean} Is the URL a Blob URL */ -const isTemporaryBlobURL = ( id, url ) => ! id && isBlobURL( url ); +const isTemporaryImage = ( id, url ) => ! id && isBlobURL( url ); /** * Is the url for the image hosted externally. An externally hosted image has no id @@ -80,7 +80,7 @@ const isTemporaryBlobURL = ( id, url ) => ! id && isBlobURL( url ); * * @return {boolean} Is the url an externally hosted url? */ -const isExternalURL = ( id, url ) => url && ! id && ! isBlobURL( url ); +const isExternalImage = ( id, url ) => url && ! id && ! isBlobURL( url ); class ImageEdit extends Component { constructor() { @@ -109,7 +109,7 @@ class ImageEdit extends Component { const { attributes, setAttributes } = this.props; const { id, url = '' } = attributes; - if ( isTemporaryBlobURL( id, url ) ) { + if ( isTemporaryImage( id, url ) ) { const file = getBlobByURL( url ); if ( file ) { @@ -128,7 +128,7 @@ class ImageEdit extends Component { const { id: prevID, url: prevURL = '' } = prevProps.attributes; const { id, url = '' } = this.props.attributes; - if ( isTemporaryBlobURL( prevID, prevURL ) && ! isTemporaryBlobURL( id, url ) ) { + if ( isTemporaryImage( prevID, prevURL ) && ! isTemporaryImage( id, url ) ) { revokeBlobURL( url ); } @@ -267,7 +267,7 @@ class ImageEdit extends Component { 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 = isExternalURL( id, url ); + const isExternal = isExternalImage( id, url ); let toolbarEditButton; if ( url ) { From a433a7aa05881204b5c33526c10226e60bd5c5b4 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 18 Oct 2018 16:53:19 +0800 Subject: [PATCH 22/24] Address code review feedback --- packages/block-library/src/image/edit.js | 6 +- .../src/components/media-placeholder/index.js | 63 ++++++++++--------- .../src/components/url-popover/index.js | 2 +- 3 files changed, 39 insertions(+), 32 deletions(-) diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js index 5471dcb0a82cc..e20e497c1122f 100644 --- a/packages/block-library/src/image/edit.js +++ b/packages/block-library/src/image/edit.js @@ -83,7 +83,7 @@ const isTemporaryImage = ( id, url ) => ! id && isBlobURL( 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 ); @@ -101,7 +101,7 @@ class ImageEdit extends Component { this.state = { captionFocused: false, - isEditing: ! this.props.attributes.url, + isEditing: ! attributes.url, }; } @@ -461,7 +461,7 @@ class ImageEdit extends Component { // Disable reason: Image itself is not meant to be // interactive, but should direct focus to block // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions - const img = ( { ); + const img = {; if ( ! isResizable || ! imageWidthWithinContainer ) { return ( diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index ad8d79a4fc12d..ba53f6ce54c0b 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -25,6 +25,30 @@ import MediaUpload from '../media-upload'; import URLPopover from '../url-popover'; import { mediaUpload } from '../../utils/'; +const InsertFromURLPopover = ( { src, onChange, onSubmit, onClose } ) => ( + + + + + + +); + class MediaPlaceholder extends Component { constructor() { super( ...arguments ); @@ -129,10 +153,9 @@ class MediaPlaceholder extends Component { const { isURLInputVisible, + src, } = this.state; - const toggleURLInput = ! isURLInputVisible ? this.openURLInput : undefined; - const allowedTypes = this.getAllowedTypes(); return ( @@ -174,38 +197,22 @@ class MediaPlaceholder extends Component { ) } /> { onSelectURL && ( -
+
{ isURLInputVisible && ( - -
- - - -
+ ) }
) } diff --git a/packages/editor/src/components/url-popover/index.js b/packages/editor/src/components/url-popover/index.js index d40874deaa9c7..c89f76d8bfd62 100644 --- a/packages/editor/src/components/url-popover/index.js +++ b/packages/editor/src/components/url-popover/index.js @@ -30,7 +30,7 @@ class URLPopover extends Component { children, renderSettings, onClose, - onClickOutside = onClose, + onClickOutside, position = 'bottom center', focusOnMount = 'firstElement', } = this.props; From df2947eda7ed427f64472769907843094b3faec5 Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 18 Oct 2018 17:11:07 +0800 Subject: [PATCH 23/24] Update changelog for blob package --- packages/blob/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/blob/CHANGELOG.md b/packages/blob/CHANGELOG.md index a49cefcd0d75c..5a0b5f114d7c3 100644 --- a/packages/blob/CHANGELOG.md +++ b/packages/blob/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.1.0 (Unreleased) + +### New Features + +- Added a new `isBlobURL` function. + ## 2.0.0 (2018-09-05) ### Breaking Change From 72f302a4090a86c00d17d045aba6c6aa500c567c Mon Sep 17 00:00:00 2001 From: Daniel Richards Date: Thu, 18 Oct 2018 17:19:15 +0800 Subject: [PATCH 24/24] Updated changelog for editor package --- packages/editor/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/editor/CHANGELOG.md b/packages/editor/CHANGELOG.md index 239f2262b15ff..251dd2ac549eb 100644 --- a/packages/editor/CHANGELOG.md +++ b/packages/editor/CHANGELOG.md @@ -8,6 +8,10 @@ - `wp.editor.PanelColor` has been deprecated in favor of `wp.editor.PanelColorSettings`. +### New Features + +- Added `onClose` prop to `URLPopover` component. + ## 4.0.0 (2018-09-30) ### Breaking Changes