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

Add/drag handle #9569

Merged
merged 30 commits into from
Sep 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
546eab5
Add drag handle icon to mover component
oandregal Sep 3, 2018
13eebbe
Add drag handle to block mover
oandregal Sep 3, 2018
380d621
Extract to own component
oandregal Sep 3, 2018
eabf09a
Hide dragHandle if block cannot be dragged
oandregal Sep 3, 2018
a430b5c
Call block onDragStar/onDragEnd methods
oandregal Sep 3, 2018
a9280d3
Change cursor to drag on the drag handle
oandregal Sep 3, 2018
79ff752
Do not use BlockDraggable
oandregal Sep 3, 2018
7a9125e
Remove no longer necessary CSS
oandregal Sep 3, 2018
22d1a4a
On dragging, set block opacity to 0.7
oandregal Sep 3, 2018
d4b3a80
Polish drag handle and metrics
Sep 4, 2018
601c575
Try: Remove hover style and make un-tabbable
Sep 4, 2018
87f2975
Set the drag/drop effect to move
oandregal Sep 4, 2018
3df86ad
Revert "Set the drag/drop effect to move"
oandregal Sep 4, 2018
6f8e5f8
Remove hover color, and make bg color friendly.
Sep 4, 2018
2475778
Remove focusable.
Sep 5, 2018
66b9d6b
Use a div as the drag handle
oandregal Sep 5, 2018
59639b1
Add tooltip
oandregal Sep 5, 2018
070b059
Expose an isDraggable property to control draggability
oandregal Sep 5, 2018
043e191
Remove empty aria label and tabindex.
Sep 6, 2018
ac3f475
Try a different style for moving.
Sep 6, 2018
dca05f5
Make the gray spot only fit the content.
Sep 6, 2018
172daa5
Update tests
oandregal Sep 6, 2018
4cbbfc5
Remove tooltip
oandregal Sep 6, 2018
638393a
Remove unnecessary blur/focus handlers
oandregal Sep 6, 2018
6901e69
Remove no longer necessary code
oandregal Sep 6, 2018
48d0d6f
Show the mover in the clone image
oandregal Sep 6, 2018
8152297
Use BlockDraggable instead of Draggable
oandregal Sep 6, 2018
42ed441
Get index and rootClientId from state
oandregal Sep 11, 2018
e9f994c
Rename arrows to icons
oandregal Sep 11, 2018
f253641
Use AccesibleSVG component
oandregal Sep 14, 2018
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
3 changes: 0 additions & 3 deletions edit-post/assets/stylesheets/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ $z-layers: (
// Active pill button
".components-button.is-button {:focus or .is-primary}": 1,

// Should have lower index than anything else positioned inside the block containers
".editor-block-list__block-draggable": 0,
Copy link
Member Author

Choose a reason for hiding this comment

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

Dragging no longer needs a dedicated DOM node. This PR removes the editor-block-list__block-draggable node from BlockDraggable.


// The draggable element should show up above the entire UI
".components-draggable__clone": 1000000000,

Expand Down
41 changes: 41 additions & 0 deletions packages/editor/src/components/block-draggable/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* WordPress dependencies
*/
import { Draggable } from '@wordpress/components';
import { withSelect } from '@wordpress/data';

const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, index, layout, onDragStart, onDragEnd } ) => {
const transferData = {
type: 'block',
fromIndex: index,
rootClientId,
clientId,
layout,
};

return (
<Draggable
elementId={ blockElementId }
transferData={ transferData }
onDragStart={ onDragStart }
onDragEnd={ onDragEnd }
>
{
( { onDraggableStart, onDraggableEnd } ) => {
return children( {
onDraggableStart: onDraggableStart,
onDraggableEnd: onDraggableEnd,
} );
}
}
</Draggable>
);
};

export default withSelect( ( select, { clientId } ) => {
const { getBlockIndex, getBlockRootClientId } = select( 'core/editor' );
return {
index: getBlockIndex( clientId ),
rootClientId: getBlockRootClientId( clientId ),
};
} )( BlockDraggable );
46 changes: 0 additions & 46 deletions packages/editor/src/components/block-list/block-draggable.js

This file was deleted.

21 changes: 6 additions & 15 deletions packages/editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import BlockContextualToolbar from './block-contextual-toolbar';
import BlockMultiControls from './multi-controls';
import BlockMobileToolbar from './block-mobile-toolbar';
import BlockInsertionPoint from './insertion-point';
import BlockDraggable from './block-draggable';
import IgnoreNestedEvents from './ignore-nested-events';
import InserterWithShortcuts from '../inserter-with-shortcuts';
import Inserter from '../inserter';
Expand Down Expand Up @@ -373,6 +372,7 @@ export class BlockListBlock extends Component {
hasSelectedInnerBlock,
isParentOfSelectedBlock,
hasMultiSelection,
isDraggable,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this prop is useless, (in the block layout as well), I don't see where it's being set?

Copy link
Member Author

@oandregal oandregal Sep 14, 2018

Choose a reason for hiding this comment

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

No, it isn't. This is a Matías' request so specific editor implementations can hide the drag handler. To maintain backwards-compatibility, unless it is specifically set to false it won't have any effect on existing editor implementations (see).

} = this.props;
const isHovered = this.state.isHovered && ! isMultiSelecting;
const { name: blockName, isValid } = block;
Expand Down Expand Up @@ -410,7 +410,7 @@ export class BlockListBlock extends Component {
'is-selected-parent': shouldAppearSelectedParent,
'is-hovered': shouldAppearHovered,
'is-reusable': isReusableBlock( blockType ),
'is-hidden': dragging,
'is-dragging': dragging,
'is-typing': isTypingWithinBlock,
'is-focused': isFocusMode && ( isSelected || isParentOfSelectedBlock ),
'is-focus-mode': isFocusMode,
Expand Down Expand Up @@ -480,18 +480,6 @@ export class BlockListBlock extends Component {
] }
{ ...wrapperProps }
>
{ ! isPartOfMultiSelection && isMovable && (
<BlockDraggable
clientId={ clientId }
rootClientId={ rootClientId }
blockElementId={ blockElementId }
layout={ layout }
order={ order }
onDragStart={ this.onDragStart }
onDragEnd={ this.onDragEnd }
isDragging={ dragging }
/>
) }
{ shouldShowInsertionPoint && (
<BlockInsertionPoint
clientId={ clientId }
Expand All @@ -509,11 +497,14 @@ export class BlockListBlock extends Component {
{ shouldRenderMovers && (
<BlockMover
clientIds={ clientId }
rootClientId={ rootClientId }
blockElementId={ blockElementId }
layout={ layout }
isFirst={ isFirst }
isLast={ isLast }
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
isDraggable={ ( isDraggable !== false ) && ( ! isPartOfMultiSelection && isMovable ) }
onDragStart={ this.onDragStart }
onDragEnd={ this.onDragEnd }
/>
) }
{ shouldShowBreadcrumb && (
Expand Down
2 changes: 2 additions & 0 deletions packages/editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ class BlockList extends Component {
isGroupedByLayout,
rootClientId,
canInsertDefaultBlock,
isDraggable,
} = this.props;

let defaultLayout;
Expand All @@ -221,6 +222,7 @@ class BlockList extends Component {
layout={ defaultLayout }
isFirst={ blockIndex === 0 }
isLast={ blockIndex === blockClientIds.length - 1 }
isDraggable={ isDraggable }
/>
) ) }
{ canInsertDefaultBlock && (
Expand Down
101 changes: 13 additions & 88 deletions packages/editor/src/components/block-list/style.scss
Original file line number Diff line number Diff line change
@@ -1,104 +1,35 @@
.editor-block-list__layout .components-draggable__clone {
& > .editor-block-list__block > .editor-block-list__block-draggable {
background: $white; // @todo: ensure this works with themes that invert the color
box-shadow: $shadow-popover;

@include break-small {
left: -$block-parent-side-ui-clearance - $border-width;
right: -$block-parent-side-ui-clearance - $border-width;

.editor-block-list__layout & {
left: -$border-width;
right: -$border-width;
}
}
}

// Hide the Block UI when dragging the block
// This ensures the page scroll properly (no sticky elements)
.editor-block-contextual-toolbar,
.editor-block-mover,
.editor-block-settings-menu {
// I think important is fine here to avoid over complexing the selector
display: none !important;
}
}

.editor-block-list__layout .editor-block-list__block-draggable {
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
z-index: z-index(".editor-block-list__block-draggable");

> .editor-block-list__block-draggable-inner {
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
visibility: hidden;

// Use opacity to work in various editor styles.
background-color: $dark-opacity-light-200;

.is-dark-theme & {
background-color: $light-opacity-light-200;
.editor-block-list__layout .editor-block-list__block.is-selected { // Needs specificity to override inherited styles.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the changes here introduced a regression in Spotlight mode.

It should be fixed here #9951

// While block is being dragged, dim the slot dragged from, and hide some UI.
&.is-dragging {
.editor-block-list__block-edit::before {
outline: none;
}

@include break-small {
margin: 0 48px;
> .editor-block-list__block-edit > * {
background: $light-gray-100;
}
}

&.is-visible > .editor-block-list__block-draggable-inner {
visibility: visible;
}

@include break-small {
// use a wider available space for hovering/selecting/dragging on top level blocks
left: -$parent-block-padding - $block-padding;
right: -$parent-block-padding - $block-padding;

// use smaller space for hovering/selecting/dragging on child blocks
.editor-block-list__layout & {
left: 0;
right: 0;
> .editor-block-list__block-edit > * > * {
visibility: hidden;
}

// Full width blocks don't have the place to expand on the side
.editor-block-list__block[data-align="full"] & {
left: 0;
right: 0;
.editor-block-mover,
.editor-block-contextual-toolbar {
display: none;
}
}


cursor: move; // Fallback for IE/Edge < 14
cursor: grab;
}


// Allow Drag & Drop when clicking on the empty area of the mover and the settings menu
.editor-block-list__layout .editor-block-list__block .editor-block-mover,
.editor-block-list__layout .editor-block-list__block .editor-block-settings-menu {
pointer-events: none;

// Nested blocks don't have any side affordance for drag and drop
.editor-block-list__layout &,
> * {
pointer-events: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Copy link
Member Author

Choose a reason for hiding this comment

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

Adds a feature by removing code

Copy link
Contributor

Choose a reason for hiding this comment

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

🤘

}
}

.editor-block-list__block {
&.is-hidden *,
&.is-hidden > * {
visibility: hidden;
}

.editor-block-list__block-edit .reusable-block-edit-panel * {
> .editor-block-list__block-edit .reusable-block-edit-panel * {
z-index: z-index(".editor-block-list__block-edit .reusable-block-edit-panel *");
}

Expand Down Expand Up @@ -438,12 +369,6 @@
float: left;
}

// There is no side UI clearance on full-wide elements, so they are simply not draggable on the sides
> .editor-block-list__block-draggable {
left: 0;
right: 0;
}

// Position hover label on the right
> .editor-block-list__breadcrumb {
right: -$border-width;
Expand Down
40 changes: 40 additions & 0 deletions packages/editor/src/components/block-mover/drag-handle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import BlockDraggable from '../block-draggable';

export const IconDragHandle = ( { isVisible, className, icon, onDragStart, onDragEnd, blockElementId, clientId, layout } ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: This component looks useless to me, we can just embed it inside the mover but I don't feel strongly.

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 find it useful to hide how the drag logic works from the block-mover itself, so the icons in the BlockMover maintain the same level of abstraction (they hide the how they do what they do).

if ( ! isVisible ) {
return null;
}

const dragHandleClassNames = classnames( 'editor-block-mover__control-drag-handle', className );

return (
<BlockDraggable
clientId={ clientId }
blockElementId={ blockElementId }
layout={ layout }
onDragStart={ onDragStart }
onDragEnd={ onDragEnd }
>
{
( { onDraggableStart, onDraggableEnd } ) => (
<div
className={ dragHandleClassNames }
aria-hidden="true"
onDragStart={ onDraggableStart }
onDragEnd={ onDraggableEnd }
draggable
>
{ icon }
</div>
) }
</BlockDraggable>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ export const downArrow = (
<polygon points="9,13.5 14.7,7.9 13.2,6.5 9,10.7 4.8,6.5 3.3,7.9 " />
</AccessibleSVG>
);

export const dragHandle = (
<AccessibleSVG width="18" height="18" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 18 18">
<path d="M13,8c0.6,0,1-0.4,1-1s-0.4-1-1-1s-1,0.4-1,1S12.4,8,13,8z M5,6C4.4,6,4,6.4,4,7s0.4,1,1,1s1-0.4,1-1S5.6,6,5,6z M5,10
c-0.6,0-1,0.4-1,1s0.4,1,1,1s1-0.4,1-1S5.6,10,5,10z M13,10c-0.6,0-1,0.4-1,1s0.4,1,1,1s1-0.4,1-1S13.6,10,13,10z M9,6
C8.4,6,8,6.4,8,7s0.4,1,1,1s1-0.4,1-1S9.6,6,9,6z M9,10c-0.6,0-1,0.4-1,1s0.4,1,1,1s1-0.4,1-1S9.6,10,9,10z" />
</AccessibleSVG>
);
Loading