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

Maintain Editable focus synchronously when splitting blocks #635

Merged
merged 3 commits into from
May 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 12 additions & 40 deletions blocks/components/editable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'element-closest';
*/
import './style.scss';
import FormatToolbar from './format-toolbar';
import TinyMCE from './tinymce';
// TODO: We mustn't import by relative path traversing from blocks to editor
// as we're doing here; instead, we should consider a common components path.
import Toolbar from '../../../editor/components/toolbar';
Expand Down Expand Up @@ -65,7 +66,6 @@ export default class Editable extends wp.element.Component {
this.onSetup = this.onSetup.bind( this );
this.onChange = this.onChange.bind( this );
this.onNewBlock = this.onNewBlock.bind( this );
this.bindEditorNode = this.bindEditorNode.bind( this );
this.onFocus = this.onFocus.bind( this );
this.onNodeChange = this.onNodeChange.bind( this );
this.onKeyDown = this.onKeyDown.bind( this );
Expand All @@ -77,28 +77,6 @@ export default class Editable extends wp.element.Component {
};
}

componentDidMount() {
this.initialize();
}

initialize() {
const config = {
target: this.editorNode,
theme: false,
inline: true,
toolbar: false,
browser_spellcheck: true,
entity_encoding: 'raw',
convert_urls: false,
setup: this.onSetup,
formats: {
strikethrough: { inline: 'del' }
}
};

tinymce.init( config );
}

onSetup( editor ) {
this.editor = editor;
editor.on( 'init', this.onInit );
Expand All @@ -110,7 +88,6 @@ export default class Editable extends wp.element.Component {
}

onInit() {
this.setContent( this.props.value );
this.focus();
}

Expand All @@ -134,7 +111,7 @@ export default class Editable extends wp.element.Component {
}

getRelativePosition( node ) {
const editorPosition = this.editorNode.closest( '.editor-visual-editor__block' ).getBoundingClientRect();
const editorPosition = this.editor.getBody().closest( '.editor-visual-editor__block' ).getBoundingClientRect();
const position = node.getBoundingClientRect();
return {
top: position.top - editorPosition.top + 40 + ( position.height ),
Expand Down Expand Up @@ -211,13 +188,10 @@ export default class Editable extends wp.element.Component {
// Splitting into two blocks
this.setContent( this.props.value );

// The setTimeout fixes the focus jump to the original block
setTimeout( () => {
this.props.onSplit(
nodeListToReact( before, createElement ),
nodeListToReact( after, createElement )
);
} );
this.props.onSplit(
nodeListToReact( before, createElement ),
nodeListToReact( after, createElement )
);
}

onNodeChange( { element, parents } ) {
Expand All @@ -242,10 +216,6 @@ export default class Editable extends wp.element.Component {
this.setState( { alignment, bookmark, formats, focusPosition } );
}

bindEditorNode( ref ) {
this.editorNode = ref;
}

updateContent() {
const bookmark = this.editor.selection.getBookmark( 2, true );
this.savedContent = this.props.value;
Expand All @@ -267,7 +237,7 @@ export default class Editable extends wp.element.Component {
}

getContent() {
return nodeListToReact( this.editorNode.childNodes || [], createElement );
return nodeListToReact( this.editor.getBody().childNodes || [], createElement );
}

focus() {
Expand Down Expand Up @@ -366,14 +336,16 @@ export default class Editable extends wp.element.Component {
}

render() {
const { tagName: Tag = 'div', style, focus, className, showAlignments = false, formattingControls } = this.props;
const { tagName, style, value, focus, className, showAlignments = false, formattingControls } = this.props;
const classes = classnames( 'blocks-editable', className );

let element = (
<Tag
ref={ this.bindEditorNode }
<TinyMCE
tagName={ tagName }
onSetup={ this.onSetup }
style={ style }
className={ classes }
defaultValue={ value }
key="editor" />
);

Expand Down
49 changes: 49 additions & 0 deletions blocks/components/editable/tinymce.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
export default class TinyMCE extends wp.element.Component {
componentDidMount() {
tinymce.init( {
target: this.editorNode,
theme: false,
inline: true,
toolbar: false,
browser_spellcheck: true,
entity_encoding: 'raw',
convert_urls: false,
setup: this.props.onSetup,
formats: {
strikethrough: { inline: 'del' }
}
} );

if ( this.props.focus ) {
this.editorNode.focus();
}
}

shouldComponentUpdate() {
// We must prevent rerenders because TinyMCE will modify the DOM, thus
// breaking React's ability to reconcile changes.
//
// See: https://github.com/facebook/react/issues/6802
return false;
}

render() {
const { tagName = 'div', style, className, defaultValue } = this.props;

// If a default value is provided, render it into the DOM even before
// TinyMCE finishes initializing. This avoids a short delay by allowing
// us to show and focus the content before it's truly ready to edit.
let children;
if ( defaultValue ) {
children = wp.element.Children.toArray( defaultValue );
}

return wp.element.createElement( tagName, {
ref: ( node ) => this.editorNode = node,
contentEditable: true,
suppressContentEditableWarning: true,
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to add, then suppress?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any reason to add, then suppress?

The suppress is needed because otherwise React will complain about rendering into a contentEditable since similar to the reason for creating this separate component, it will lead to a bad time if reconciliation is attempted.

style,
className
}, children );
}
}
17 changes: 15 additions & 2 deletions editor/modes/visual-editor/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class VisualEditorBlock extends wp.element.Component {
this.setAttributes = this.setAttributes.bind( this );
this.maybeDeselect = this.maybeDeselect.bind( this );
this.maybeHover = this.maybeHover.bind( this );
this.maybeStartTyping = this.maybeStartTyping.bind( this );
this.mergeWithPrevious = this.mergeWithPrevious.bind( this );
this.previousOffset = null;
}
Expand Down Expand Up @@ -63,6 +64,18 @@ class VisualEditorBlock extends wp.element.Component {
}
}

maybeStartTyping() {
// We do not want to dispatch start typing if...
// - State value already reflects that we're typing (dispatch noise)
// - The current block is not selected (e.g. after a split occurs,
// we'll still receive the keyDown event, but the focus has since
// shifted to the newly created block)
const { isTyping, isSelected, onStartTyping } = this.props;
if ( ! isTyping && isSelected ) {
onStartTyping();
}
}

mergeWithPrevious() {
const { block, previousBlock, onFocus, replaceBlocks } = this.props;

Expand Down Expand Up @@ -137,7 +150,7 @@ class VisualEditorBlock extends wp.element.Component {
'is-hovered': isHovered
} );

const { onSelect, onStartTyping, onHover, onMouseLeave, onFocus, onInsertAfter } = this.props;
const { onSelect, onHover, onMouseLeave, onFocus, onInsertAfter } = this.props;

// Determine whether the block has props to apply to the wrapper
let wrapperProps;
Expand Down Expand Up @@ -177,7 +190,7 @@ class VisualEditorBlock extends wp.element.Component {
<Slot name="Formatting.Toolbar" />
</div>
}
<div onKeyDown={ isTyping ? null : onStartTyping }>
<div onKeyDown={ this.maybeStartTyping }>
<BlockEdit
focus={ focus }
attributes={ block.attributes }
Expand Down