From ab9dab2f7867afb5c6cc0be16f180b2fa769e677 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Sun, 7 Apr 2019 21:40:30 -0400 Subject: [PATCH 1/8] Add ability to style image caption Adds ability to style image caption with bold, italic, links, and strikethrough (wordpress-mobile/gutenberg-mobile#574). Adds ability for image captions to wrap to multiple lines instead of getting cut off (wordpress-mobile/gutenberg-mobile#590). --- .../block-library/src/image/edit.native.js | 49 ++++++++++++++++--- .../src/image/styles.native.scss | 4 -- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index af26201f1d35d..601c90e3d7b3a 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -62,6 +62,7 @@ class ImageEdit extends React.Component { progress: 0, isUploadInProgress: false, isUploadFailed: false, + captionFocused: false }; this.mediaUpload = this.mediaUpload.bind( this ); @@ -73,6 +74,8 @@ class ImageEdit extends React.Component { this.onSetLinkDestination = this.onSetLinkDestination.bind( this ); this.onImagePressed = this.onImagePressed.bind( this ); this.onClearSettings = this.onClearSettings.bind( this ); + this.onFocusCaption = this.onFocusCaption.bind( this ); + this.onBlurCaption = this.onBlurCaption.bind( this ); } componentDidMount() { @@ -108,6 +111,8 @@ class ImageEdit extends React.Component { } else if ( attributes.id && ! isURL( attributes.url ) ) { requestImageFailedRetryDialog( attributes.id ); } + + this._caption.blur() } mediaUpload( payload ) { @@ -209,6 +214,28 @@ class ImageEdit extends React.Component { ]; } + onFocusCaption() { + if (this.props.onFocus) { + this.props.onFocus() + } + if ( ! this.state.captionFocused ) { + this.setState( { + captionFocused: true, + } ); + } + } + + onBlurCaption() { + if (this.props.onBlur) { + this.props.onBlur() + } + if ( this.state.captionFocused ) { + this.setState( { + captionFocused: false, + } ); + } + } + render() { const { attributes, isSelected, setAttributes } = this.props; const { url, caption, height, width, alt, href } = attributes; @@ -384,14 +411,20 @@ class ImageEdit extends React.Component { { ( ! RichText.isEmpty( caption ) > 0 || isSelected ) && ( - setAttributes( { caption: newCaption } ) } - /> + { + this._caption = ref + } } + tagName="p" + placeholder={ __( 'Write caption…' ) } + value={ caption } + onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } + onFocus={ this.onFocusCaption } + onBlur={ this.onBlurCaption } + isSelected={ isSelected && this.state.captionFocused } + fontSize={ 14 } + underlineColorAndroid="transparent" + /> ) } diff --git a/packages/block-library/src/image/styles.native.scss b/packages/block-library/src/image/styles.native.scss index b187d12aa32fe..fc4e82aded534 100644 --- a/packages/block-library/src/image/styles.native.scss +++ b/packages/block-library/src/image/styles.native.scss @@ -18,10 +18,6 @@ align-items: center; } -.caption-text { - font-family: $default-regular-font; -} - .clearSettingsButton { color: $alert-red; } From b597c6e138664a034603dcc9a8335c8cea3b3398 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Mon, 8 Apr 2019 08:17:51 -0400 Subject: [PATCH 2/8] Address style errors from linter --- .../block-library/src/image/edit.native.js | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index 601c90e3d7b3a..649da17a2b6c4 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -2,7 +2,7 @@ * External dependencies */ import React from 'react'; -import { View, ImageBackground, TextInput, Text, TouchableWithoutFeedback } from 'react-native'; +import { View, ImageBackground, Text, TouchableWithoutFeedback } from 'react-native'; import { subscribeMediaUpload, requestMediaPickFromMediaLibrary, @@ -62,7 +62,7 @@ class ImageEdit extends React.Component { progress: 0, isUploadInProgress: false, isUploadFailed: false, - captionFocused: false + captionFocused: false, }; this.mediaUpload = this.mediaUpload.bind( this ); @@ -112,7 +112,7 @@ class ImageEdit extends React.Component { requestImageFailedRetryDialog( attributes.id ); } - this._caption.blur() + this._caption.blur(); } mediaUpload( payload ) { @@ -215,8 +215,8 @@ class ImageEdit extends React.Component { } onFocusCaption() { - if (this.props.onFocus) { - this.props.onFocus() + if ( this.props.onFocus ) { + this.props.onFocus(); } if ( ! this.state.captionFocused ) { this.setState( { @@ -226,8 +226,8 @@ class ImageEdit extends React.Component { } onBlurCaption() { - if (this.props.onBlur) { - this.props.onBlur() + if ( this.props.onBlur ) { + this.props.onBlur(); } if ( this.state.captionFocused ) { this.setState( { @@ -411,20 +411,20 @@ class ImageEdit extends React.Component { { ( ! RichText.isEmpty( caption ) > 0 || isSelected ) && ( - { - this._caption = ref - } } - tagName="p" - placeholder={ __( 'Write caption…' ) } - value={ caption } - onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } - onFocus={ this.onFocusCaption } - onBlur={ this.onBlurCaption } - isSelected={ isSelected && this.state.captionFocused } - fontSize={ 14 } - underlineColorAndroid="transparent" - /> + { + this._caption = ref; + } } + tagName="p" + placeholder={ __( 'Write caption…' ) } + value={ caption } + onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } + onFocus={ this.onFocusCaption } + onBlur={ this.onBlurCaption } + isSelected={ isSelected && this.state.captionFocused } + fontSize={ 14 } + underlineColorAndroid="transparent" + /> ) } From b4afb1dba2e851a34ab886d44fecf936d823def9 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Mon, 8 Apr 2019 08:57:38 -0400 Subject: [PATCH 3/8] Hide image toolbar controls if caption selected --- packages/block-library/src/image/edit.native.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index 649da17a2b6c4..019dd56aa26f9 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -358,9 +358,11 @@ class ImageEdit extends React.Component { { showSpinner && } - - { toolbarEditButton } - + { ( ! this.state.captionFocused ) && + + { toolbarEditButton } + + } Date: Mon, 8 Apr 2019 20:28:05 -0400 Subject: [PATCH 4/8] Update state in componentDidMount The previous approach of setting the child component's `isSelected` prop at the time the child was rendered based on `this.props.isSelected && this.state.isCaptionSelected` did not work when: (a) the child was selected (so its `isSelected` prop was true); (b) the child component had no text; and (c) the parent's `isSelected` prop was changed to false (i.e., another block was selected). Because the child component is not rendered when both the parent's `isSelected` prop is false and the caption does not contain any text, the child component's `onBlur` prop function (the `onCaptionBlur function) would not be called and update the `isCaptionSelected` state to be false. This bug shows when a user selects an empty caption, then selects a different block (thereby hiding the caption since it is empty), and then re-selects the image with the empty caption. In that scenario, upon re-selecting the image with the empty caption, the empty caption would appear and immediately be selected instead of just appearing. This bug would not appear if there was any text in the caption, because then the caption would always render and its `onBlur` prop function would be called. --- .../block-library/src/image/edit.native.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index 019dd56aa26f9..0033ba3173a2e 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -62,7 +62,7 @@ class ImageEdit extends React.Component { progress: 0, isUploadInProgress: false, isUploadFailed: false, - captionFocused: false, + isCaptionSelected: false, }; this.mediaUpload = this.mediaUpload.bind( this ); @@ -103,6 +103,12 @@ class ImageEdit extends React.Component { this.removeMediaUploadListener(); } + componentWillReceiveProps( nextProps ) { + this.setState( ( state ) => ( { + isCaptionSelected: nextProps.isSelected && state.isCaptionSelected, + } ) ); + } + onImagePressed() { const { attributes } = this.props; @@ -218,9 +224,9 @@ class ImageEdit extends React.Component { if ( this.props.onFocus ) { this.props.onFocus(); } - if ( ! this.state.captionFocused ) { + if ( ! this.state.isCaptionSelected ) { this.setState( { - captionFocused: true, + isCaptionSelected: true, } ); } } @@ -229,9 +235,9 @@ class ImageEdit extends React.Component { if ( this.props.onBlur ) { this.props.onBlur(); } - if ( this.state.captionFocused ) { + if ( this.state.isCaptionSelected ) { this.setState( { - captionFocused: false, + isCaptionSelected: false, } ); } } @@ -358,7 +364,7 @@ class ImageEdit extends React.Component { { showSpinner && } - { ( ! this.state.captionFocused ) && + { ( ! this.state.isCaptionSelected ) && { toolbarEditButton } @@ -423,7 +429,7 @@ class ImageEdit extends React.Component { onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } onFocus={ this.onFocusCaption } onBlur={ this.onBlurCaption } - isSelected={ isSelected && this.state.captionFocused } + isSelected={ this.state.isCaptionSelected } fontSize={ 14 } underlineColorAndroid="transparent" /> From 5d3f86e674e3830acbf38ff32c90ae3b774b6ce0 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Wed, 10 Apr 2019 20:32:30 -0400 Subject: [PATCH 5/8] Add comment explaining isSelected guard --- packages/block-library/src/image/edit.native.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index 0033ba3173a2e..83d0c34126a00 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -104,6 +104,8 @@ class ImageEdit extends React.Component { } componentWillReceiveProps( nextProps ) { + // Avoid a UI flicker in the toolbar by insuring that isCaptionSelected + // is updated immediately any time the isSelected prop becomes false this.setState( ( state ) => ( { isCaptionSelected: nextProps.isSelected && state.isCaptionSelected, } ) ); From e85aa9814babb44749649c21eeca057c6c6c3cbe Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Sun, 14 Apr 2019 20:29:38 -0400 Subject: [PATCH 6/8] Update image caption tagname from p to figcaption This change brings mobile's handling of image captinos in line with the web. It also addresses a crash that was occurring when the enter key was tapped to enter a new line in an image caption. --- packages/block-library/src/image/edit.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index 83d0c34126a00..dd61c6eb92254 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -425,7 +425,7 @@ class ImageEdit extends React.Component { setRef={ ( ref ) => { this._caption = ref; } } - tagName="p" + tagName="figcaption" placeholder={ __( 'Write caption…' ) } value={ caption } onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } From 4c4801a48fa98192b5d3b2ad4389215ce9388fb0 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Tue, 16 Apr 2019 08:13:07 -0400 Subject: [PATCH 7/8] Remove `onBlurCaption` function On iOS the display of the link UI modal was causing the `onBlurCaption` function to be called, which would update the image caption's `isSelected` prop to false. That would, in turn, immediately remove the just-displayed modal. Restructuring the logic to not like this to not use the image caption's `onBlur` function avoids that issue. --- packages/block-library/src/image/edit.native.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index dd61c6eb92254..9d2a69f47f786 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -75,7 +75,6 @@ class ImageEdit extends React.Component { this.onImagePressed = this.onImagePressed.bind( this ); this.onClearSettings = this.onClearSettings.bind( this ); this.onFocusCaption = this.onFocusCaption.bind( this ); - this.onBlurCaption = this.onBlurCaption.bind( this ); } componentDidMount() { @@ -121,6 +120,9 @@ class ImageEdit extends React.Component { } this._caption.blur(); + this.setState( { + isCaptionSelected: false, + } ); } mediaUpload( payload ) { @@ -233,17 +235,6 @@ class ImageEdit extends React.Component { } } - onBlurCaption() { - if ( this.props.onBlur ) { - this.props.onBlur(); - } - if ( this.state.isCaptionSelected ) { - this.setState( { - isCaptionSelected: false, - } ); - } - } - render() { const { attributes, isSelected, setAttributes } = this.props; const { url, caption, height, width, alt, href } = attributes; @@ -430,7 +421,7 @@ class ImageEdit extends React.Component { value={ caption } onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) } onFocus={ this.onFocusCaption } - onBlur={ this.onBlurCaption } + onBlur={ this.props.onBlur } // always assign onBlur as props isSelected={ this.state.isCaptionSelected } fontSize={ 14 } underlineColorAndroid="transparent" From 91c366296b2d2f7f7c1e541049d323e1c1543836 Mon Sep 17 00:00:00 2001 From: Matt Chowning Date: Thu, 25 Apr 2019 21:57:11 -0400 Subject: [PATCH 8/8] Remove tagName from caption RichText component Explicitly setting the tagName to figcaption caused an invalid block to be saved if a caption ended with an empty line. --- .../src/components/rich-text/index.native.js | 23 ++++++++++++------- .../block-library/src/image/edit.native.js | 1 - 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/index.native.js b/packages/block-editor/src/components/rich-text/index.native.js index 99a65f60396ad..af6947db8435d 100644 --- a/packages/block-editor/src/components/rich-text/index.native.js +++ b/packages/block-editor/src/components/rich-text/index.native.js @@ -110,6 +110,7 @@ export class RichText extends Component { this.onSelectionChange = this.onSelectionChange.bind( this ); this.valueToFormat = this.valueToFormat.bind( this ); this.willTrimSpaces = this.willTrimSpaces.bind( this ); + this.getHtmlToRender = this.getHtmlToRender.bind( this ); this.state = { start: 0, end: 0, @@ -643,6 +644,19 @@ export class RichText extends Component { return false; } + getHtmlToRender( record, tagName ) { + // Save back to HTML from React tree + const value = this.valueToFormat( record ); + + if ( value === undefined || value === '' ) { + this.lastEventCount = undefined; // force a refresh on the native side + return ''; + } else if ( tagName ) { + return `<${ tagName }>${ value }`; + } + return value; + } + render() { const { tagName, @@ -653,14 +667,7 @@ export class RichText extends Component { } = this.props; const record = this.getRecord(); - // Save back to HTML from React tree - const value = this.valueToFormat( record ); - let html = `<${ tagName }>${ value }`; - // We need to check if the value is undefined or empty, and then assign it properly otherwise the placeholder is not visible - if ( value === undefined || value === '' ) { - html = ''; - this.lastEventCount = undefined; // force a refresh on the native side - } + const html = this.getHtmlToRender( record, tagName ); let minHeight = styles[ 'block-editor-rich-text' ].minHeight; if ( style && style.minHeight ) { diff --git a/packages/block-library/src/image/edit.native.js b/packages/block-library/src/image/edit.native.js index c17a147ea1808..a6c7b4f9a09f9 100644 --- a/packages/block-library/src/image/edit.native.js +++ b/packages/block-library/src/image/edit.native.js @@ -429,7 +429,6 @@ class ImageEdit extends React.Component { setRef={ ( ref ) => { this._caption = ref; } } - tagName="figcaption" placeholder={ __( 'Write caption…' ) } value={ caption } onChange={ ( newCaption ) => setAttributes( { caption: newCaption } ) }