From e04ef6add87f40aea9a1fcc432eeda3870609e34 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 7 Feb 2018 13:26:29 +1000 Subject: [PATCH 1/5] Indicate which text is being turned into a link Insert a placeholder so that it is clear which text will be made into a link when creating a link using the RichText component. --- blocks/rich-text/format-toolbar/index.js | 36 +++++++++++++++--------- blocks/url-input/index.js | 3 -- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index edc1c54a61b74..0ed18b0b2e120 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -15,6 +15,12 @@ import { filterURLForDisplay } from '../../../editor/utils/url'; const { ESCAPE, LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } = keycodes; +/** + * When inserting a new link, we insert an tag with this placeholder href + * so that there is a visual indication of which text will be made into a link. + */ +const NEW_LINK_PLACEHOLDER_VALUE = '_wp_link_placeholder'; + const FORMATTING_CONTROLS = [ { icon: 'editor-bold', @@ -49,7 +55,6 @@ class FormatToolbar extends Component { super( ...arguments ); this.state = { - isAddingLink: false, isEditingLink: false, newLinkValue: '', }; @@ -77,7 +82,6 @@ class FormatToolbar extends Component { componentWillReceiveProps( nextProps ) { if ( this.props.selectedNodeId !== nextProps.selectedNodeId ) { this.setState( { - isAddingLink: false, isEditingLink: false, newLinkValue: '', } ); @@ -97,25 +101,27 @@ class FormatToolbar extends Component { } addLink() { - this.setState( { isEditingLink: false, isAddingLink: true, newLinkValue: '' } ); + this.props.onChange( { link: { value: NEW_LINK_PLACEHOLDER_VALUE } } ); } dropLink() { this.props.onChange( { link: undefined } ); - this.setState( { isEditingLink: false, isAddingLink: false, newLinkValue: '' } ); } editLink( event ) { event.preventDefault(); - this.setState( { isEditingLink: false, isAddingLink: true, newLinkValue: this.props.formats.link.value } ); + this.setState( { isEditingLink: true, newLinkValue: this.props.formats.link.value } ); } submitLink( event ) { event.preventDefault(); - this.props.onChange( { link: { value: this.state.newLinkValue } } ); - if ( this.state.isAddingLink ) { + this.setState( { isEditingLink: false, newLinkValue: '' } ); + + if ( this.props.formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ) { this.props.speak( __( 'Link added.' ), 'assertive' ); } + + this.props.onChange( { link: { value: this.state.newLinkValue } } ); } isFormatActive( format ) { @@ -124,10 +130,7 @@ class FormatToolbar extends Component { render() { const { formats, focusPosition, enabledControls = DEFAULT_CONTROLS, customControls = [] } = this.props; - const { isAddingLink, isEditingLink, newLinkValue } = this.state; - const linkStyle = focusPosition ? - { position: 'absolute', ...focusPosition } : - null; + const { isEditingLink, newLinkValue } = this.state; const toolbarControls = FORMATTING_CONTROLS.concat( customControls ) .filter( control => enabledControls.indexOf( control.format ) !== -1 ) @@ -136,15 +139,20 @@ class FormatToolbar extends Component { return { ...control, onClick: isLink ? this.addLink : this.toggleFormat( control.format ), - isActive: this.isFormatActive( control.format ) || ( isLink && isAddingLink ), + isActive: this.isFormatActive( control.format ), }; } ); + const hasEditLinkUI = formats.link && ( isEditingLink || formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ); + const hasViewLinkUI = formats.link && ! isEditingLink && formats.link.value !== NEW_LINK_PLACEHOLDER_VALUE; + + const linkStyle = focusPosition ? { position: 'absolute', ...focusPosition } : null; + return (
- { ( isAddingLink || isEditingLink ) && + { hasEditLinkUI && // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ @@ -164,7 +172,7 @@ class FormatToolbar extends Component { /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ } - { !! formats.link && ! isAddingLink && ! isEditingLink && + { hasViewLinkUI && // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar /* eslint-disable jsx-a11y/no-static-element-interactions */ diff --git a/blocks/url-input/index.js b/blocks/url-input/index.js index a98d0df14025a..755707fae0b2c 100644 --- a/blocks/url-input/index.js +++ b/blocks/url-input/index.js @@ -177,11 +177,9 @@ class UrlInput extends Component { render() { const { value, instanceId } = this.props; const { showSuggestions, posts, selectedSuggestion, loading } = this.state; - /* eslint-disable jsx-a11y/no-autofocus */ return (
); - /* eslint-enable jsx-a11y/no-autofocus */ } } From 2b593d2fd55b305f911175087b77f8076ebae82f Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 7 Feb 2018 13:38:27 +1000 Subject: [PATCH 2/5] Use link toolbar button to remove links Consolidate the dedicated 'remove link' button into the toolbar button that adds a link. --- blocks/rich-text/format-toolbar/index.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index 0ed18b0b2e120..9354330069c7f 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -38,8 +38,6 @@ const FORMATTING_CONTROLS = [ format: 'strikethrough', }, { - icon: 'admin-links', - title: __( 'Link' ), format: 'link', }, ]; @@ -133,13 +131,24 @@ class FormatToolbar extends Component { const { isEditingLink, newLinkValue } = this.state; const toolbarControls = FORMATTING_CONTROLS.concat( customControls ) - .filter( control => enabledControls.indexOf( control.format ) !== -1 ) + .filter( control => enabledControls.includes( control.format ) ) .map( ( control ) => { - const isLink = control.format === 'link'; + const isActive = this.isFormatActive( control.format ); + + if ( control.format === 'link' ) { + return { + ...control, + icon: isActive ? 'editor-unlink' : 'admin-links', // TODO: proper unlink icon + title: isActive ? __( 'Unlink' ) : __( 'Link' ), + isActive, + onClick: isActive ? this.dropLink : this.addLink, + }; + } + return { ...control, - onClick: isLink ? this.addLink : this.toggleFormat( control.format ), - isActive: this.isFormatActive( control.format ), + isActive, + onClick: this.toggleFormat( control.format ), }; } ); @@ -165,7 +174,6 @@ class FormatToolbar extends Component {
-
@@ -190,7 +198,6 @@ class FormatToolbar extends Component { { formats.link.value && filterURLForDisplay( decodeURI( formats.link.value ) ) } -
From 5d31aff4bd514f5e1dd6e56bdaa8a7e6337dc300 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 7 Feb 2018 14:06:46 +1000 Subject: [PATCH 3/5] Improve reliability of ESCAPE and ENTER when editing links When editing or adding a link, ESCAPE should cancel editing mode and ENTER should submit the changes. --- blocks/rich-text/format-toolbar/index.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index 9354330069c7f..a83d2e727c6e9 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -13,7 +13,7 @@ import './style.scss'; import UrlInput from '../../url-input'; import { filterURLForDisplay } from '../../../editor/utils/url'; -const { ESCAPE, LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } = keycodes; +const { ESCAPE } = keycodes; /** * When inserting a new link, we insert an tag with this placeholder href @@ -66,15 +66,15 @@ class FormatToolbar extends Component { } onKeyDown( event ) { + event.stopPropagation(); + if ( event.keyCode === ESCAPE ) { - if ( this.state.isEditingLink ) { - event.stopPropagation(); + this.setState( { isEditingLink: false, newLinkValue: '' } ); + + if ( this.props.formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ) { this.dropLink(); } } - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) { - stopKeyPropagation( event ); - } } componentWillReceiveProps( nextProps ) { @@ -113,6 +113,7 @@ class FormatToolbar extends Component { submitLink( event ) { event.preventDefault(); + this.setState( { isEditingLink: false, newLinkValue: '' } ); if ( this.props.formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ) { From 1ccbcb8fd5e8c0478e9d804513e77c04a5154e84 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 7 Feb 2018 16:17:56 +1000 Subject: [PATCH 4/5] Expand link UI to fit contents The link popup should widen to fit the URL within it. --- blocks/rich-text/format-toolbar/index.js | 81 +++++++++++----------- blocks/rich-text/format-toolbar/style.scss | 8 +-- blocks/rich-text/index.js | 3 +- 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index a83d2e727c6e9..d2b5105735b80 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -153,56 +153,55 @@ class FormatToolbar extends Component { }; } ); - const hasEditLinkUI = formats.link && ( isEditingLink || formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ); - const hasViewLinkUI = formats.link && ! isEditingLink && formats.link.value !== NEW_LINK_PLACEHOLDER_VALUE; - - const linkStyle = focusPosition ? { position: 'absolute', ...focusPosition } : null; + const hasLinkUI = !! formats.link; + const hasEditLinkUI = hasLinkUI && ( isEditingLink || formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ); + const hasViewLinkUI = hasLinkUI && ! isEditingLink && formats.link.value !== NEW_LINK_PLACEHOLDER_VALUE; return (
- { hasEditLinkUI && - // 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 */ - } - - { hasViewLinkUI && - // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar - /* eslint-disable jsx-a11y/no-static-element-interactions */ + { hasLinkUI && -
-
- + { hasEditLinkUI && + // 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 */ + } + + { hasViewLinkUI && + // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar + /* eslint-disable jsx-a11y/no-static-element-interactions */ + + +
+ /* eslint-enable jsx-a11y/no-static-element-interactions */ + }
- /* eslint-enable jsx-a11y/no-static-element-interactions */ }
); diff --git a/blocks/rich-text/format-toolbar/style.scss b/blocks/rich-text/format-toolbar/style.scss index 609255d2e3ffc..7f761c080a66c 100644 --- a/blocks/rich-text/format-toolbar/style.scss +++ b/blocks/rich-text/format-toolbar/style.scss @@ -3,11 +3,11 @@ } .blocks-format-toolbar__link-modal { - position: absolute; + position: relative; + left: -50%; box-shadow: 0px 3px 20px rgba( 18, 24, 30, .1 ), 0px 1px 3px rgba( 18, 24, 30, .1 ); border: 1px solid #e0e5e9; background: #fff; - width: 300px; display: flex; flex-direction: column; font-family: $default-font; @@ -37,8 +37,4 @@ position: relative; white-space: nowrap; min-width: 0; - - &:after { - @include long-content-fade( $size: 40% ); - } } diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index eeeebff9581f1..0927eba587a0d 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -428,11 +428,10 @@ export default class RichText extends Component { const container = findRelativeParent( this.editor.getBody() ); const containerPosition = container.getBoundingClientRect(); const toolbarOffset = { top: 10, left: 0 }; - const linkModalWidth = 298; return { top: position.top - containerPosition.top + ( position.height ) + toolbarOffset.top, - left: position.left - containerPosition.left - ( linkModalWidth / 2 ) + ( position.width / 2 ) + toolbarOffset.left, + left: position.left - containerPosition.left + ( position.width / 2 ) + toolbarOffset.left, }; } From 3d9bc7bea5042b3192ad8e0832ef0abe4c96159a Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 8 Feb 2018 13:45:23 +1000 Subject: [PATCH 5/5] Remove placeholder links when selection changes Remove the placeholder link inserted when a new link is added when the user moves the selection elsewhere. --- blocks/rich-text/format-toolbar/index.js | 17 +++------ blocks/rich-text/index.js | 47 ++++++++++++++++++------ 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/blocks/rich-text/format-toolbar/index.js b/blocks/rich-text/format-toolbar/index.js index d2b5105735b80..b53cf3cd3336c 100644 --- a/blocks/rich-text/format-toolbar/index.js +++ b/blocks/rich-text/format-toolbar/index.js @@ -11,16 +11,11 @@ import { keycodes } from '@wordpress/utils'; */ import './style.scss'; import UrlInput from '../../url-input'; +import { LINK_PLACEHOLDER_VALUE } from '../'; import { filterURLForDisplay } from '../../../editor/utils/url'; const { ESCAPE } = keycodes; -/** - * When inserting a new link, we insert an tag with this placeholder href - * so that there is a visual indication of which text will be made into a link. - */ -const NEW_LINK_PLACEHOLDER_VALUE = '_wp_link_placeholder'; - const FORMATTING_CONTROLS = [ { icon: 'editor-bold', @@ -71,7 +66,7 @@ class FormatToolbar extends Component { if ( event.keyCode === ESCAPE ) { this.setState( { isEditingLink: false, newLinkValue: '' } ); - if ( this.props.formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ) { + if ( this.props.formats.link.value === LINK_PLACEHOLDER_VALUE ) { this.dropLink(); } } @@ -99,7 +94,7 @@ class FormatToolbar extends Component { } addLink() { - this.props.onChange( { link: { value: NEW_LINK_PLACEHOLDER_VALUE } } ); + this.props.onChange( { link: { value: LINK_PLACEHOLDER_VALUE } } ); } dropLink() { @@ -116,7 +111,7 @@ class FormatToolbar extends Component { this.setState( { isEditingLink: false, newLinkValue: '' } ); - if ( this.props.formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ) { + if ( this.props.formats.link.value === LINK_PLACEHOLDER_VALUE ) { this.props.speak( __( 'Link added.' ), 'assertive' ); } @@ -154,8 +149,8 @@ class FormatToolbar extends Component { } ); const hasLinkUI = !! formats.link; - const hasEditLinkUI = hasLinkUI && ( isEditingLink || formats.link.value === NEW_LINK_PLACEHOLDER_VALUE ); - const hasViewLinkUI = hasLinkUI && ! isEditingLink && formats.link.value !== NEW_LINK_PLACEHOLDER_VALUE; + const hasEditLinkUI = hasLinkUI && ( isEditingLink || formats.link.value === LINK_PLACEHOLDER_VALUE ); + const hasViewLinkUI = hasLinkUI && ! isEditingLink && formats.link.value !== LINK_PLACEHOLDER_VALUE; return (
diff --git a/blocks/rich-text/index.js b/blocks/rich-text/index.js index 0927eba587a0d..8d7a783142a9e 100644 --- a/blocks/rich-text/index.js +++ b/blocks/rich-text/index.js @@ -72,6 +72,12 @@ export function getFormatProperties( formatName, parents ) { const DEFAULT_FORMATS = [ 'bold', 'italic', 'strikethrough', 'link' ]; +/** + * When inserting a new link, we insert an tag with this placeholder href + * so that there is a visual indication of which text will be made into a link. + */ +export const LINK_PLACEHOLDER_VALUE = '_wp_link_placeholder'; + export default class RichText extends Component { constructor( props ) { super( ...arguments ); @@ -655,22 +661,41 @@ export default class RichText extends Component { ); } - onNodeChange( { parents } ) { + removePlaceholderLinks() { + this.editor.$( 'a' ).each( ( index, element ) => { + if ( element.getAttribute( 'href' ) === LINK_PLACEHOLDER_VALUE ) { + this.editor.dom.remove( element, true ); + } + } ); + } + + getFormats( parents ) { + return this.editor.formatter.matchAll( this.props.formattingControls ).reduce( ( formats, format ) => { + formats[ format ] = { + isActive: true, + ...getFormatProperties( format, parents ), + }; + return formats; + }, {} ); + } + + onNodeChange( event ) { if ( document.activeElement !== this.editor.getBody() ) { return; } - const formatNames = this.props.formattingControls; - const formats = this.editor.formatter.matchAll( formatNames ).reduce( ( accFormats, activeFormat ) => { - accFormats[ activeFormat ] = { - isActive: true, - ...getFormatProperties( activeFormat, parents ), - }; - return accFormats; - }, {} ); + const formats = this.getFormats( event.parents ); + + // Remove all placeholder links if selection moves away from a placeholder link + if ( ! formats.link || formats.link.value !== LINK_PLACEHOLDER_VALUE ) { + this.removePlaceholderLinks(); + } - const focusPosition = this.getFocusPosition(); - this.setState( { formats, focusPosition, selectedNodeId: this.state.selectedNodeId + 1 } ); + this.setState( { + formats, + focusPosition: this.getFocusPosition(), + selectedNodeId: this.state.selectedNodeId + 1, + } ); } updateContent() {