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

Allow drag to different page #3312

Merged
merged 37 commits into from
Oct 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
6e37b45
Allow drag to different page
barklund Sep 20, 2019
ba20251
Refactor duplicated logic
barklund Sep 20, 2019
354443b
Remove unnecessary hook
barklund Sep 20, 2019
40259f8
More performant structure
barklund Sep 23, 2019
9fd43c3
Better separation of concerns
barklund Sep 23, 2019
305b692
Handle CTA blocks correctly when dragging between pages
barklund Sep 23, 2019
f7d0c19
Fix proptypes and add inline comment
barklund Sep 23, 2019
83b2e4f
Fix offset bug when dragging 2+ pages left
barklund Sep 23, 2019
3928f60
Adding documentation for functions
barklund Sep 24, 2019
efd22fd
Fix disallow duplicate CTA
barklund Sep 24, 2019
95ca6c8
Generalised helper for checking if block is allowed on given page
barklund Sep 26, 2019
6109bec
Refactored `isCTABlock` to helpers
barklund Sep 26, 2019
b3be9f5
Rename variable and move to constants
barklund Sep 27, 2019
0395f30
Merge branch 'develop' into feature/3311-allow-drag-to-page
barklund Sep 27, 2019
04fffea
Make props required where applicable
barklund Sep 27, 2019
3c7afcf
Reorganised helper to aid in testing
barklund Sep 27, 2019
aee7550
Fix typos
barklund Sep 27, 2019
64f341d
Reuse `STORY_PAGE_MARGIN` where applicable
barklund Sep 27, 2019
5160c2f
Missing rewrite to `isCTABlock` fixed
barklund Sep 27, 2019
9bcb884
Fix minor errors in rewrites
barklund Sep 27, 2019
7ee41f3
Add `aria-grabbed` to the block mover
barklund Sep 27, 2019
3b8a139
Revert "Add `aria-grabbed` to the block mover"
barklund Sep 27, 2019
4f4f619
Fix test wording
barklund Sep 30, 2019
d1813e1
Updated component documentation
barklund Oct 1, 2019
c3e51f4
Updated logic to display non-allowed drop zones
barklund Oct 1, 2019
4041030
Add border on allowed regions instead (and fix css lint)
barklund Oct 1, 2019
1cde15c
Major rewrite of entire implementation
barklund Oct 2, 2019
3f13272
Merge branch 'develop' into feature/3311-allow-drag-to-page
barklund Oct 3, 2019
e963844
Convert helper function to hook
barklund Oct 3, 2019
3241e3b
Added dependencies for `useSelect`
barklund Oct 3, 2019
005bac0
Created new constant for the height of the drop zone for CTAs
barklund Oct 3, 2019
354ee52
Updated `useMoveBlockToPage` and added tests
barklund Oct 3, 2019
8f068be
Fix proptype missed in refactor
barklund Oct 3, 2019
d5eb6a8
Merge branch 'develop' into feature/3311-allow-drag-to-page
swissspidy Oct 4, 2019
144c430
Refactor MediaInserter to use hooks
swissspidy Oct 4, 2019
60f0198
Move isBlockAllowedOnPage function inline
swissspidy Oct 4, 2019
9bd9ab3
Remove ifCondition HOC usage from stories editor
swissspidy Oct 4, 2019
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
39 changes: 39 additions & 0 deletions assets/src/stories-editor/blocks/amp-story-page/edit.css
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,45 @@
opacity: .5;
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover {
opacity: 1;
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-droppable::before,
.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-amp-amp-story-cta.amp-page-draggable-hover-droppable::before {
display: block;
content: "";
position: absolute;
top: -4px;
bottom: -4px;
left: -4px;
right: -4px;
border: 4px solid theme(primary);
z-index: 10;
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-amp-amp-story-cta.amp-page-draggable-hover-droppable::before {
top: calc(80% - 2px);
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-undroppable::after,
.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-amp-amp-story-cta.amp-page-draggable-hover-droppable::after {
display: block;
content: "";
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
background-color: #fff;
opacity: .8;
z-index: 10;
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-draggable-hover-amp-amp-story-cta.amp-page-draggable-hover-droppable::after {
bottom: calc(20% + 2px);
}

.editor-block-list__layout [data-type="amp/amp-story-page"].amp-page-inactive > div {
pointer-events: none;
}
Expand Down
5 changes: 3 additions & 2 deletions assets/src/stories-editor/components/block-mover/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ In addition the component also uses default `IgnoreNestedEvents` component which
## Unchanged files.

The following files of the component are 100% or almost unchanged:
- block-draggable.js:
- This file is mainly copied from core, the only difference is switching to using internal Draggable
- ignore-nested-events.js (unchanged)

## Modified files
Expand All @@ -19,6 +17,9 @@ draggable.js: This file has been modified to display the clone in relation to th
- The clone styling has been changed to ignore the % values and to match the size of the original element;
- Resizing the dragged clone has been removed.

block-draggable.js: This file has been extended to allow drag between pages
- Assumes that blocks are only draggable inside pages, not inside other types of elements

block-drag-area.js: This was renamed from from drag-handle.js
- Now wraps draggable element(s) as children, whereas it used to appear alongside them.
- Enables dragging the entire block.
Expand Down
106 changes: 90 additions & 16 deletions assets/src/stories-editor/components/block-mover/block-draggable.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,110 @@ import PropTypes from 'prop-types';
/**
* WordPress dependencies
*/
import { withSelect } from '@wordpress/data';
import { useSelect } from '@wordpress/data';
import { useRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import { useIsBlockAllowedOnPage, useMoveBlockToPage } from '../../helpers';
import Draggable from './draggable';

const BlockDraggable = ( { children, clientId, blockName, rootClientId, blockElementId, index, onDragStart, onDragEnd } ) => {
const BlockDraggable = ( { children, clientId, blockName, blockElementId, onDragStart, onDragEnd } ) => {
const { rootClientId } = useSelect( ( select ) => select( 'core/block-editor' ).getBlockRootClientId( clientId ) );
const { index } = useSelect( ( select ) => select( 'core/block-editor' ).getBlockIndex( clientId, rootClientId ) );

const transferData = {
type: 'block',
srcIndex: index,
srcRootClientId: rootClientId,
srcClientId: clientId,
};

const { moveBlockToPage, getPageByOffset } = useMoveBlockToPage( clientId );

const isBlockAllowedOnPage = useIsBlockAllowedOnPage();

// This holds the currently highlighted element, if any
const hoverElement = useRef( { pageId: null, element: null, classes: [] } );

/**
* Clear highlighting of pages.
*/
const clearHighlight = () => {
// Unhighlight old highlighted page.
if ( hoverElement.current.element ) {
hoverElement.current.element.classList.remove( ...hoverElement.current.classes );
}

// Reset current hover element
hoverElement.current = { pageId: null, element: null, classes: [] };
barklund marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* On the page given by the offset, set classes indicating that an element drag on this page is in progress, which element is being dragged and whether a drop is allowed on this page.
*
* @param {number} offset Integer specifying offset from current page - e.g. -2 on page 3 will return id of page 1. Page indices are zero-based.
*/
const setHighlightByOffset = ( offset ) => {
// Get neighboring page.
const pageId = getPageByOffset( offset );
const hasHighlightChanged = pageId !== hoverElement.current.pageId;

if ( ! hasHighlightChanged ) {
return;
}

clearHighlight();

// Highlight page and mark whether drop is allowed or not.
if ( pageId ) {
// Drop is always allowed on the initial page (offset=0)
const isAllowed = offset === 0 || isBlockAllowedOnPage( blockName, pageId );
const classes = [
barklund marked this conversation as resolved.
Show resolved Hide resolved
'amp-page-draggable-hover',
`amp-page-draggable-hover-${ blockName.replace( /\W/g, '-' ) }`,
isAllowed ? 'amp-page-draggable-hover-droppable' : 'amp-page-draggable-hover-undroppable',
];

const element = document.getElementById( `block-${ pageId }` );
element.classList.add( ...classes );

hoverElement.current = { element, pageId, classes };
}
};

/**
* Drop this element on the page `offset` away from the current page (if possible). If dropped, update the position of the element on the new page.
*
* Currently this function removes the old element and creates a clone on the new page.
*
* @param {number} offset Integer specifying offset from current page - e.g. -2 on page 3 will return id of page 1. Offset must be non-zero.
* @param {Object} newAttributes Object with attributes to update on element on new page.
*/
const dropElementByOffset = ( offset, newAttributes ) => {
barklund marked this conversation as resolved.
Show resolved Hide resolved
const pageId = getPageByOffset( offset );
if ( ! pageId ) {
return;
}

if ( ! isBlockAllowedOnPage( blockName, pageId ) ) {
return;
}

moveBlockToPage( pageId, newAttributes );
};

return (
<Draggable
blockName={ blockName }
elementId={ blockElementId }
transferData={ transferData }
onDragStart={ onDragStart }
onDragEnd={ onDragEnd }
clearHighlight={ clearHighlight }
setHighlightByOffset={ setHighlightByOffset }
dropElementByOffset={ dropElementByOffset }
>
{
( { onDraggableStart, onDraggableEnd } ) => {
Expand All @@ -46,21 +128,13 @@ const BlockDraggable = ( { children, clientId, blockName, rootClientId, blockEle
};

BlockDraggable.propTypes = {
index: PropTypes.number.isRequired,
rootClientId: PropTypes.string,
clientId: PropTypes.string,
blockElementId: PropTypes.string,
blockName: PropTypes.string,
children: PropTypes.node.isRequired,
clientId: PropTypes.string.isRequired,
blockElementId: PropTypes.string.isRequired,
blockName: PropTypes.string.isRequired,
children: PropTypes.any.isRequired,
onDragStart: PropTypes.func,
onDragEnd: PropTypes.func,
};

export default withSelect( ( select, { clientId } ) => {
const { getBlockIndex, getBlockRootClientId } = select( 'core/block-editor' );
const rootClientId = getBlockRootClientId( clientId );
return {
index: getBlockIndex( clientId, rootClientId ),
rootClientId,
};
} )( BlockDraggable );
export default BlockDraggable;

83 changes: 73 additions & 10 deletions assets/src/stories-editor/components/block-mover/draggable.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,21 @@ import { withSafeTimeout } from '@wordpress/compose';
/**
* Internal dependencies
*/
import { getPixelsFromPercentage } from '../../helpers';
import {
getPixelsFromPercentage,
getPercentageFromPixels,
getRelativeElementPosition,
isCTABlock,
} from '../../helpers';
import {
STORY_PAGE_INNER_WIDTH,
STORY_PAGE_INNER_HEIGHT,
STORY_PAGE_INNER_HEIGHT_FOR_CTA,
STORY_PAGE_MARGIN,
} from '../../constants';

const PAGE_AND_MARGIN = STORY_PAGE_INNER_WIDTH + STORY_PAGE_MARGIN;

const { Image, navigator } = window;

const cloneWrapperClass = 'components-draggable__clone';
Expand Down Expand Up @@ -61,13 +70,41 @@ class Draggable extends Component {
* @param {Object} event The non-custom DragEvent.
*/
onDragEnd = ( event ) => {
const { onDragEnd = noop } = this.props;
const { clearHighlight, dropElementByOffset, blockName, setTimeout, onDragEnd = noop } = this.props;
if ( event ) {
event.preventDefault();
}

// Make sure to clear highlight.
clearHighlight();

// Attempt drop on neighbor if offset
if ( this.pageOffset !== 0 ) {
// All this is about calculating the position of the (correct) element on the new page.
const currentElementTop = parseInt( this.cloneWrapper.style.top );
const currentElementLeft = parseInt( this.cloneWrapper.style.left );
const newLeft = currentElementLeft - ( this.pageOffset * PAGE_AND_MARGIN );

let baseHeight, xAttribute, yAttribute;
if ( isCTABlock( blockName ) ) {
baseHeight = STORY_PAGE_INNER_HEIGHT_FOR_CTA;
xAttribute = 'btnPositionLeft';
yAttribute = 'btnPositionTop';
} else {
baseHeight = STORY_PAGE_INNER_HEIGHT;
xAttribute = 'positionLeft';
yAttribute = 'positionTop';
}

const newAttributes = {
[ xAttribute ]: getPercentageFromPixels( 'x', newLeft, STORY_PAGE_INNER_WIDTH ),
[ yAttribute ]: getPercentageFromPixels( 'y', currentElementTop, baseHeight ),
};
dropElementByOffset( this.pageOffset, newAttributes );
}

this.resetDragState();
this.props.setTimeout( onDragEnd );
setTimeout( onDragEnd );
}

/**
Expand All @@ -76,10 +113,11 @@ class Draggable extends Component {
* @param {Object} event The non-custom DragEvent.
*/
onDragOver = ( event ) => {
const { setHighlightByOffset, blockName } = this.props;
const top = parseInt( this.cloneWrapper.style.top ) + event.clientY - this.cursorTop;

// Don't allow the CTA button to go over its top limit.
if ( 'amp/amp-story-cta' === this.props.blockName ) {
if ( isCTABlock( blockName ) ) {
barklund marked this conversation as resolved.
Show resolved Hide resolved
this.cloneWrapper.style.top = top >= 0 ? `${ top }px` : '0px';
} else {
this.cloneWrapper.style.top = `${ top }px`;
Expand All @@ -91,6 +129,21 @@ class Draggable extends Component {
// Update cursor coordinates.
this.cursorLeft = event.clientX;
this.cursorTop = event.clientY;

// Check if mouse (*not* element, but actual cursor) is over neighboring page to either side.
const currentElementLeft = parseInt( this.cloneWrapper.style.left );
const cursorLeftRelativeToPage = currentElementLeft + this.cursorLeftInsideElement;
const isOffRight = cursorLeftRelativeToPage > PAGE_AND_MARGIN;
const isOffLeft = cursorLeftRelativeToPage < -STORY_PAGE_MARGIN;
this.pageOffset = 0;
if ( isOffLeft || isOffRight ) {
// Check how far off we are to that side - on large screens you can drag elements 2+ pages over to either side.
this.pageOffset = ( isOffLeft ?
-Math.ceil( ( -STORY_PAGE_MARGIN - cursorLeftRelativeToPage ) / PAGE_AND_MARGIN ) :
Math.ceil( ( cursorLeftRelativeToPage - PAGE_AND_MARGIN ) / PAGE_AND_MARGIN )
);
}
setHighlightByOffset( this.pageOffset );
}

onDrop = () => {
Expand All @@ -108,9 +161,9 @@ class Draggable extends Component {
*/
onDragStart = ( event ) => {
const { blockName, elementId, transferData, onDragStart = noop } = this.props;
const isCTABlock = 'amp/amp-story-cta' === blockName;
const blockIsCTA = isCTABlock( blockName );
// In the CTA block only the inner element (the button) is draggable, not the whole block.
const element = isCTABlock ? document.getElementById( elementId ) : document.getElementById( elementId ).parentNode;
const element = blockIsCTA ? document.getElementById( elementId ) : document.getElementById( elementId ).parentNode;

if ( ! element ) {
event.preventDefault();
Expand Down Expand Up @@ -149,11 +202,18 @@ class Draggable extends Component {
this.cloneWrapper.style.transform = clone.style.transform;

// 20% of the full value in case of CTA block.
const baseHeight = isCTABlock ? STORY_PAGE_INNER_HEIGHT / 5 : STORY_PAGE_INNER_HEIGHT;
const baseHeight = blockIsCTA ? STORY_PAGE_INNER_HEIGHT_FOR_CTA : STORY_PAGE_INNER_HEIGHT;

// Position clone over the original element.
this.cloneWrapper.style.top = `${ getPixelsFromPercentage( 'y', parseInt( clone.style.top ), baseHeight ) }px`;
this.cloneWrapper.style.left = `${ getPixelsFromPercentage( 'x', parseInt( clone.style.left ), STORY_PAGE_INNER_WIDTH ) }px`;
const top = getPixelsFromPercentage( 'y', parseInt( clone.style.top ), baseHeight );
const left = getPixelsFromPercentage( 'x', parseInt( clone.style.left ), STORY_PAGE_INNER_WIDTH );
this.cloneWrapper.style.top = `${ top }px`;
this.cloneWrapper.style.left = `${ left }px`;

// Get starting position information
const absolutePositionOfPage = getRelativeElementPosition( elementWrapper, document.documentElement );
const absoluteElementLeft = absolutePositionOfPage.left + left;
this.cursorLeftInsideElement = event.clientX - absoluteElementLeft;

clone.id = `clone-${ elementId }`;
clone.style.top = 0;
Expand Down Expand Up @@ -229,8 +289,11 @@ Draggable.propTypes = {
} ),
onDragStart: PropTypes.func,
onDragEnd: PropTypes.func,
clearHighlight: PropTypes.func.isRequired,
setHighlightByOffset: PropTypes.func.isRequired,
dropElementByOffset: PropTypes.func.isRequired,
setTimeout: PropTypes.func.isRequired,
children: PropTypes.node.isRequired,
children: PropTypes.func.isRequired,
};

export default withSafeTimeout( Draggable );
19 changes: 8 additions & 11 deletions assets/src/stories-editor/components/block-navigation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import PropTypes from 'prop-types';
/**
* WordPress dependencies
*/
import { withSelect, useSelect, useDispatch } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';
import { DropZoneProvider, NavigableMenu } from '@wordpress/components';
import { compose, ifCondition } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -96,6 +95,12 @@ function BlockNavigation() {
};
}, [] );

const isReordering = useSelect( ( select ) => select( 'amp/story' ).isReordering(), [] );

if ( isReordering ) {
return null;
}

const hasBlocks = blocks.length > 0 || unMovableBlock;

return (
Expand All @@ -122,13 +127,5 @@ function BlockNavigation() {
);
}

export default compose(
withSelect( ( select ) => {
const { isReordering } = select( 'amp/story' );
export default BlockNavigation;

return {
isReordering: isReordering(),
};
} ),
ifCondition( ( { isReordering } ) => ! isReordering ),
)( BlockNavigation );
Loading