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

Block List: Extract scroll preservation as non-visual component #5085

Merged
merged 2 commits into from
Mar 7, 2018
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
2 changes: 2 additions & 0 deletions edit-post/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
EditorNotices,
PostPublishPanel,
DocumentTitle,
PreserveScrollInReorder,
} from '@wordpress/editor';

/**
Expand Down Expand Up @@ -83,6 +84,7 @@ function Layout( {
<Header />
<div className="edit-post-layout__content" role="region" aria-label={ __( 'Editor content' ) } tabIndex="-1">
<EditorNotices />
<PreserveScrollInReorder />
<div className="edit-post-layout__editor">
<EditorModeKeyboardShortcuts />
{ mode === 'text' && <TextEditor /> }
Expand Down
25 changes: 1 addition & 24 deletions editor/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { Component, findDOMNode, compose } from '@wordpress/element';
import {
keycodes,
focus,
getScrollContainer,
placeCaretAtHorizontalEdge,
placeCaretAtVerticalEdge,
} from '@wordpress/utils';
Expand Down Expand Up @@ -100,8 +99,6 @@ export class BlockListBlock extends Component {
this.onClick = this.onClick.bind( this );
this.selectOnOpen = this.selectOnOpen.bind( this );
this.onSelectionChange = this.onSelectionChange.bind( this );

this.previousOffset = null;
this.hadTouchStart = false;

this.state = {
Expand Down Expand Up @@ -140,31 +137,12 @@ export class BlockListBlock extends Component {
}

componentWillReceiveProps( newProps ) {
if (
this.props.order !== newProps.order &&
( newProps.isSelected || newProps.isFirstMultiSelected )
) {
this.previousOffset = this.node.getBoundingClientRect().top;
}

if ( newProps.isTyping || newProps.isSelected ) {
this.hideHoverEffects();
}
}

componentDidUpdate( prevProps ) {
// Preserve scroll prosition when block rearranged
if ( this.previousOffset ) {
const scrollContainer = getScrollContainer( this.node );
if ( scrollContainer ) {
scrollContainer.scrollTop = scrollContainer.scrollTop +
this.node.getBoundingClientRect().top -
this.previousOffset;
}

this.previousOffset = null;
}

// Bind or unbind mousemove from page when user starts or stops typing
if ( this.props.isTyping !== prevProps.isTyping ) {
if ( this.props.isTyping ) {
Expand Down Expand Up @@ -201,8 +179,7 @@ export class BlockListBlock extends Component {

bindBlockNode( node ) {
// Disable reason: The block element uses a component to manage event
// nesting, but we rely on a raw DOM node for focusing and preserving
// scroll offset on move.
// nesting, but we rely on a raw DOM node for focusing.
//
// eslint-disable-next-line react/no-find-dom-node
this.node = findDOMNode( node );
Expand Down
1 change: 1 addition & 0 deletions editor/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export { default as Inserter } from './inserter';
export { default as MultiBlocksSwitcher } from './block-switcher/multi-blocks-switcher';
export { default as MultiSelectScrollIntoView } from './multi-select-scroll-into-view';
export { default as NavigableToolbar } from './navigable-toolbar';
export { default as PreserveScrollInReorder } from './preserve-scroll-in-reorder';
export { default as Warning } from './warning';
export { default as WritingFlow } from './writing-flow';

Expand Down
7 changes: 6 additions & 1 deletion editor/components/multi-select-scroll-into-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import { Component } from '@wordpress/element';
import { withSelect } from '@wordpress/data';
import { getScrollContainer } from '@wordpress/utils';

/**
* Internal dependencies
*/
import { getBlockDOMNode } from '../../utils/dom';

class MultiSelectScrollIntoView extends Component {
componentDidUpdate() {
// Relies on expectation that `componentDidUpdate` will only be called
Expand All @@ -29,7 +34,7 @@ class MultiSelectScrollIntoView extends Component {
return;
}

const extentNode = document.querySelector( '[data-block="' + extentUID + '"]' );
const extentNode = getBlockDOMNode( extentUID );
if ( ! extentNode ) {
return;
}
Expand Down
80 changes: 80 additions & 0 deletions editor/components/preserve-scroll-in-reorder/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { withSelect } from '@wordpress/data';
import { getScrollContainer } from '@wordpress/utils';

/**
* Internal dependencies
*/
import { getBlockDOMNode } from '../../utils/dom';

/**
* Non-visual component which preserves offset of selected block within nearest
* scrollable container while reordering.
*
* @example
*
* ```jsx
* <PreserveScrollInReorder />
* ```
*/
class PreserveScrollInReorder extends Component {
componentWillUpdate( nextProps ) {
const { blockOrder, selectionStart } = nextProps;
if ( blockOrder !== this.props.blockOrder && selectionStart ) {
this.setPreviousOffset( selectionStart );
}
}

componentDidUpdate() {
if ( this.previousOffset ) {
this.restorePreviousOffset();
}
}

/**
* Given the block UID of the start of the selection, saves the block's
* top offset as an instance property before a reorder is to occur.
*
* @param {string} selectionStart UID of selected block.
*/
setPreviousOffset( selectionStart ) {
const blockNode = getBlockDOMNode( selectionStart );
if ( ! blockNode ) {
return;
}

this.previousOffset = blockNode.getBoundingClientRect().top;
}

/**
* After a block reordering, restores the previous viewport top offset.
*/
restorePreviousOffset() {
const { selectionStart } = this.props;
const blockNode = getBlockDOMNode( selectionStart );
if ( blockNode ) {
const scrollContainer = getScrollContainer( blockNode );
if ( scrollContainer ) {
scrollContainer.scrollTop = scrollContainer.scrollTop +
blockNode.getBoundingClientRect().top -
this.previousOffset;
}
}

delete this.previousOffset;
}

render() {
return null;
}
}

export default withSelect( ( select ) => {
return {
blockOrder: select( 'core/editor' ).getBlockOrder(),
selectionStart: select( 'core/editor' ).getBlockSelectionStart(),
};
} )( PreserveScrollInReorder );
26 changes: 26 additions & 0 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,32 @@ export function getBlockCount( state, rootUID ) {
return getBlockOrder( state, rootUID ).length;
}

/**
* Returns the current block selection start. This value may be null, and it
* may represent either a singular block selection or multi-selection start.
* A selection is singular if its start and end match.
*
* @param {Object} state Global application state.
*
* @return {?string} UID of block selection start.
*/
export function getBlockSelectionStart( state ) {
return state.blockSelection.start;
}

/**
* Returns the current block selection end. This value may be null, and it
* may represent either a singular block selection or multi-selection end.
* A selection is singular if its start and end match.
*
* @param {Object} state Global application state.
*
* @return {?string} UID of block selection end.
*/
export function getBlockSelectionEnd( state ) {
return state.blockSelection.end;
}

/**
* Returns the number of blocks currently selected in the post.
*
Expand Down
12 changes: 12 additions & 0 deletions editor/utils/dom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Given a block UID, returns the corresponding DOM node for the block, if
* exists. As much as possible, this helper should be avoided, and used only
* in cases where isolated behaviors need remote access to a block node.
*
* @param {string} uid Block UID.
*
* @return {Element} Block DOM node.
*/
export function getBlockDOMNode( uid ) {
return document.querySelector( '[data-block="' + uid + '"]' );
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing two other places where we use a similar technical to get the block DOM node, should we update those as well? (search for [data-block=")

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm seeing two other places where we use a similar technical to get the block DOM node, should we update those as well? (search for [data-block=")

I could only find one other, but replaced it in fad28e2.

There's a reference to data-block in WritingFlow as well, but this wouldn't be as easy to replace and is proposed for removal anyways in #5289.

}