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

Display block boundaries when cursor moves while typing #1149

Merged
merged 3 commits into from
Jun 13, 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
28 changes: 28 additions & 0 deletions editor/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,31 @@ export function mergeBlocks( blockA, blockB ) {
blocks: [ blockA, blockB ],
};
}

/**
* Returns an action object used in signalling that the user has begun to type
* within a block's editable field.
*
* @param {String} uid Block UID
* @return {Object} Action object
*/
export function startTypingInBlock( uid ) {
return {
type: 'START_TYPING',
uid,
};
}

/**
* Returns an action object used in signalling that the user has stopped typing
* within a block's editable field.
*
* @param {String} uid Block UID
* @return {Object} Action object
*/
export function stopTypingInBlock( uid ) {
return {
type: 'STOP_TYPING',
uid,
};
}
85 changes: 57 additions & 28 deletions editor/modes/visual-editor/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
mergeBlocks,
insertBlock,
clearSelectedBlock,
startTypingInBlock,
stopTypingInBlock,
} from '../../actions';
import {
getPreviousBlock,
Expand All @@ -50,16 +52,18 @@ class VisualEditorBlock extends wp.element.Component {
this.setAttributes = this.setAttributes.bind( this );
this.maybeHover = this.maybeHover.bind( this );
this.maybeStartTyping = this.maybeStartTyping.bind( this );
this.stopTyping = this.stopTyping.bind( this );
this.removeOrDeselect = this.removeOrDeselect.bind( this );
this.mergeBlocks = this.mergeBlocks.bind( this );
this.onFocus = this.onFocus.bind( this );
this.onPointerDown = this.onPointerDown.bind( this );
this.previousOffset = null;
}

bindBlockNode( node ) {
this.node = node;
this.props.blockRef( node );
componentDidMount() {
if ( this.props.focus ) {
this.node.focus();
}
}

componentWillReceiveProps( newProps ) {
Expand All @@ -72,6 +76,45 @@ class VisualEditorBlock extends wp.element.Component {
}
}

componentDidUpdate( prevProps ) {
// Preserve scroll prosition when block rearranged
if ( this.previousOffset ) {
window.scrollTo(
window.scrollX,
window.scrollY + this.node.getBoundingClientRect().top - this.previousOffset
);
this.previousOffset = null;
}

// Focus node when focus state is programmatically transferred.
if ( this.props.focus && ! prevProps.focus ) {
this.node.focus();
}

// Bind or unbind mousemove from page when user starts or stops typing
const { isTyping } = this.props;
if ( isTyping !== prevProps.isTyping ) {
if ( isTyping ) {
document.addEventListener( 'mousemove', this.stopTyping );
} else {
this.removeStopTypingListener();
}
}
}

componentWillUnmount() {
this.removeStopTypingListener();
}

removeStopTypingListener() {
document.removeEventListener( 'mousemove', this.stopTyping );
}

bindBlockNode( node ) {
this.node = node;
this.props.blockRef( node );
}

setAttributes( attributes ) {
const { block, onChange } = this.props;
onChange( block.uid, {
Expand Down Expand Up @@ -104,6 +147,10 @@ class VisualEditorBlock extends wp.element.Component {
}
}

stopTyping() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could avoid this function

Copy link
Member Author

@aduth aduth Jun 13, 2017

Choose a reason for hiding this comment

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

You mean binding this.props.onStopTying as the event handler? It's a little concerning to me since the onStopTyping prop may not be the same in the constructor as it is when the component later unmounts (or the event is bound), so we risk zombie event listeners. I'm actually unsure of the behavior of react-redux's mapDispatchToProps. Seems like it should only need to generate the props once when initializing, so it's likely not a concern in reality, just in theory.

this.props.onStopTyping();
}

removeOrDeselect( { keyCode, target } ) {
const {
uid,
Expand Down Expand Up @@ -153,27 +200,6 @@ class VisualEditorBlock extends wp.element.Component {
}
}

componentDidUpdate( prevProps ) {
if ( this.previousOffset ) {
window.scrollTo(
window.scrollX,
window.scrollY + this.node.getBoundingClientRect().top - this.previousOffset
);
this.previousOffset = null;
}

// Focus node when focus state is programmatically transferred.
if ( this.props.focus && ! prevProps.focus ) {
this.node.focus();
}
}

componentDidMount() {
if ( this.props.focus ) {
this.node.focus();
}
}

onFocus( event ) {
if ( event.target === this.node ) {
this.props.onSelect();
Expand Down Expand Up @@ -318,12 +344,15 @@ export default connect(
onDeselect() {
dispatch( clearSelectedBlock() );
},

onStartTyping() {
dispatch( {
type: 'START_TYPING',
uid: ownProps.uid,
} );
dispatch( startTypingInBlock( ownProps.uid ) );
},

onStopTyping() {
dispatch( stopTypingInBlock( ownProps.uid ) );
},

onHover() {
dispatch( {
type: 'TOGGLE_BLOCK_HOVERED',
Expand Down
10 changes: 10 additions & 0 deletions editor/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,16 @@ export function selectedBlock( state = {}, action ) {
typing: true,
};

case 'STOP_TYPING':
if ( action.uid !== state.uid ) {
return state;
}

return {
...state,
typing: false,
};

case 'REPLACE_BLOCKS':
if ( ! action.blocks || ! action.blocks.length || action.uids.indexOf( state.uid ) === -1 ) {
return state;
Expand Down
25 changes: 24 additions & 1 deletion editor/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import { expect } from 'chai';
/**
* Internal dependencies
*/
import { focusBlock, replaceBlocks } from '../actions';
import {
focusBlock,
replaceBlocks,
startTypingInBlock,
stopTypingInBlock,
} from '../actions';

describe( 'actions', () => {
describe( 'focusBlock', () => {
Expand Down Expand Up @@ -36,4 +41,22 @@ describe( 'actions', () => {
} );
} );
} );

describe( 'startTypingInBlock', () => {
it( 'should return the START_TYPING action', () => {
expect( startTypingInBlock( 'chicken' ) ).to.eql( {
type: 'START_TYPING',
uid: 'chicken',
} );
} );
} );

describe( 'stopTypingInBlock', () => {
it( 'should return the STOP_TYPING action', () => {
expect( stopTypingInBlock( 'chicken' ) ).to.eql( {
type: 'STOP_TYPING',
uid: 'chicken',
} );
} );
} );
} );
23 changes: 23 additions & 0 deletions editor/test/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,29 @@ describe( 'state', () => {
expect( state ).to.eql( { uid: 'chicken', typing: true, focus: {} } );
} );

it( 'should do nothing if typing stopped not within selected block', () => {
const original = selectedBlock( undefined, {} );
const state = selectedBlock( original, {
type: 'STOP_TYPING',
uid: 'chicken',
} );

expect( state ).to.equal( original );
} );

it( 'should reset typing flag if typing stopped within selected block', () => {
const original = selectedBlock( undefined, {
type: 'START_TYPING',
uid: 'chicken',
} );
const state = selectedBlock( original, {
type: 'STOP_TYPING',
uid: 'chicken',
} );

expect( state ).to.eql( { uid: 'chicken', typing: false, focus: {} } );
} );

it( 'should set the typing flag and merge the existing state', () => {
const original = deepFreeze( { uid: 'ribs', typing: false, focus: { editable: 'citation' } } );
const state = selectedBlock( original, {
Expand Down