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.

Sign up for GitHub

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

Support drag-and-drop for submenus of navigation blocks #24479

Merged
merged 4 commits into from
Aug 20, 2020
Merged

Support drag-and-drop for submenus of navigation blocks #24479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6fa1f13
Support drag and drop for submenus in Navigation block
kevin940726 Aug 7, 2020
0901929
Add a bunch of CSS to handle dragging style properly
kevin940726 Aug 11, 2020
16be7f0
Extract useIsDraggingWithin into its own hook
kevin940726 Aug 13, 2020
6057e8f
Add jsdoc
kevin940726 Aug 13, 2020
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: 1 addition & 2 deletions packages/block-editor/src/components/block-draggable/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

// This hides the block being dragged.
// The effect is that the block being dragged appears to be "lifted".
.is-dragging.is-selected,
.is-dragging.is-multi-selected {
.is-dragging {
draganescu marked this conversation as resolved.
Show resolved Hide resolved
display: none !important;
}
6 changes: 4 additions & 2 deletions packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ function BlockListBlock( {
{
'wp-block': ! isAligned,
'has-warning': ! isValid || !! hasError || isUnregisteredBlock,
'is-selected': isSelected,
// Don't select the item while dragging.
'is-selected': isSelected && ! isDragging,
'is-highlighted': isHighlighted,
'is-multi-selected': isMultiSelected,
'is-reusable': isReusableBlock( blockType ),
Expand All @@ -159,7 +160,8 @@ function BlockListBlock( {
'is-focused':
isFocusMode && ( isSelected || isAncestorOfSelectedBlock ),
'is-focus-mode': isFocusMode,
'has-child-selected': isAncestorOfSelectedBlock,
// Don't select child while dragging.
'has-child-selected': isAncestorOfSelectedBlock && ! isDragging,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure the comments here add much, I think we could either remove them or add some more detail on why those classes aren't applied when dragging.

},
className
);
Expand Down
11 changes: 11 additions & 0 deletions packages/block-editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,15 @@
}
}

// Hide the popover block editor list while dragging.
// Using a hacky animation to delay hiding the element so that dragging can still work.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention that if the element is hidden immediately, dragging won't work, and this is why a delay is required.

@keyframes hide-during-dragging {
to {
position: fixed;
transform: translate(9999px, 9999px);
}
}

.components-popover.block-editor-block-list__block-popover {
z-index: z-index(".block-editor-block-list__block-popover");
position: absolute;
Expand Down Expand Up @@ -657,6 +666,8 @@

.is-dragging-components-draggable & {
opacity: 0;
// Use a minimal duration to delay hiding the element, see hide-during-dragging animation for more details.
Copy link
Contributor

@talldan talldan Aug 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It might be good to explain in the comment why the toolbar/popover needs to be hidden, mentioning that it obscures drag events if it remains in position overlapping preceding blocks.

animation: hide-during-dragging 1ms linear forwards;
}
}

Expand Down
81 changes: 76 additions & 5 deletions packages/block-library/src/navigation-link/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import { escape, get, head, find } from 'lodash';
*/
import { compose } from '@wordpress/compose';
import { createBlock } from '@wordpress/blocks';
import { useDispatch, withDispatch, withSelect } from '@wordpress/data';
import {
useSelect,
useDispatch,
withDispatch,
withSelect,
} from '@wordpress/data';
import {
KeyboardShortcuts,
PanelBody,
Expand Down Expand Up @@ -39,6 +44,54 @@ import { link as linkIcon } from '@wordpress/icons';
*/
import { ToolbarSubmenuIcon, ItemSubmenuIcon } from './icons';

/**
* A React hook to determine if it's dragging within the target element.
*
* @typedef {import('@wordpress/element').RefObject} RefObject
*
* @param {RefObject<HTMLElement>} elementRef The target elementRef object.
*
* @return {boolean} Is dragging within the target element.
*/
const useIsDraggingWithin = ( elementRef ) => {
const [ isDraggingWithin, setIsDraggingWithin ] = useState( false );

useEffect( () => {
function handleDragStart( event ) {
// Check the first time when the dragging starts.
handleDragEnter( event );
}

// Set to false whenever the user cancel the drag event by either releasing the mouse or press Escape.
function handleDragEnd() {
setIsDraggingWithin( false );
}

function handleDragEnter( event ) {
// Check if the current target is inside the item element.
if ( elementRef.current.contains( event.target ) ) {
setIsDraggingWithin( true );
} else {
setIsDraggingWithin( false );
}
}

// Bind these events to the document to catch all drag events.
// Ideally, we can also use `event.relatedTarget`, but sadly that doesn't work in Safari.
document.addEventListener( 'dragstart', handleDragStart );
document.addEventListener( 'dragend', handleDragEnd );
document.addEventListener( 'dragenter', handleDragEnter );

return () => {
document.removeEventListener( 'dragstart', handleDragStart );
document.removeEventListener( 'dragend', handleDragEnd );
document.removeEventListener( 'dragenter', handleDragEnter );
};
}, [] );

return isDraggingWithin;
};

function NavigationLinkEdit( {
attributes,
hasDescendants,
Expand All @@ -65,9 +118,16 @@ function NavigationLinkEdit( {
};
const { saveEntityRecord } = useDispatch( 'core' );
const [ isLinkOpen, setIsLinkOpen ] = useState( false );
const listItemRef = useRef( null );
const isDraggingWithin = useIsDraggingWithin( listItemRef );
const itemLabelPlaceholder = __( 'Add link…' );
const ref = useRef();

const isDraggingBlocks = useSelect(
( select ) => select( 'core/block-editor' ).isDraggingBlocks(),
[]
);

// Show the LinkControl on mount if the URL is empty
// ( When adding a new menu item)
// This can't be done in the useState call because it cconflicts
Expand Down Expand Up @@ -183,8 +243,13 @@ function NavigationLinkEdit( {
</InspectorControls>
<Block.li
className={ classnames( {
'is-editing': isSelected || isParentOfSelectedBlock,
'is-selected': isSelected,
'is-editing':
( isSelected || isParentOfSelectedBlock ) &&
// Don't show the element as editing while dragging.
! isDraggingBlocks,
// Don't select the element while dragging.
'is-selected': isSelected && ! isDraggingBlocks,
'is-dragging-within': isDraggingWithin,
'has-link': !! url,
'has-child': hasDescendants,
'has-text-color': rgbTextColor,
Expand All @@ -196,6 +261,7 @@ function NavigationLinkEdit( {
color: rgbTextColor,
backgroundColor: rgbBackgroundColor,
} }
ref={ listItemRef }
>
<div className="wp-block-navigation-link__content">
<RichText
Expand Down Expand Up @@ -282,7 +348,9 @@ function NavigationLinkEdit( {
renderAppender={
( isSelected && hasDescendants ) ||
( isImmediateParentOfSelectedBlock &&
! selectedBlockHasDescendants )
! selectedBlockHasDescendants ) ||
// Show the appender while dragging to allow inserting element between item and the appender.
( isDraggingBlocks && hasDescendants )
? InnerBlocks.DefaultAppender
: false
}
Expand All @@ -292,7 +360,10 @@ function NavigationLinkEdit( {
className: classnames(
'wp-block-navigation__container',
{
'is-parent-of-selected-block': isParentOfSelectedBlock,
'is-parent-of-selected-block':
isParentOfSelectedBlock &&
// Don't select as parent of selected block while dragging.
! isDraggingBlocks,
}
),
} }
Expand Down
20 changes: 15 additions & 5 deletions packages/block-library/src/navigation/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,27 @@ $navigation-item-height: 46px;
}

&.is-selected,
&.has-child-selected {
&.has-child-selected,
// Show the submenu list when is dragging and drag enter the list item.
body.is-dragging-components-draggable &.is-dragging-within {
> .wp-block-navigation__container {
opacity: 1;
visibility: visible;
}
}
}

body.is-dragging-components-draggable .wp-block-navigation-link > .wp-block-navigation__container {
Comment on lines +52 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's drop the body part of the selector if possible, usually just class names are used.

// Set opacity to 1 to still be able to show the draggable chip.
opacity: 1;
visibility: hidden;

// Show the chip but hide the submenu list.
.block-editor-block-draggable-chip-wrapper {
visibility: visible;
}
}

/**
* Colors Selector component
*/
Expand Down Expand Up @@ -95,7 +108,7 @@ $colors-selector-size: 22px;
min-width: $colors-selector-size;
height: $colors-selector-size;
min-height: $colors-selector-size;
line-height: ($colors-selector-size - 2);
line-height: ( $colors-selector-size - 2 );
padding: 2px;

> svg {
Expand Down Expand Up @@ -145,7 +158,6 @@ $color-control-label-height: 20px;
}

.wp-block-navigation-placeholder {

.components-spinner {
margin-top: -4px;
margin-left: 4px;
Expand All @@ -165,7 +177,6 @@ $color-control-label-height: 20px;

// Styles for when there are Menus present in the dropdown.
.components-custom-select-control.has-menus .components-custom-select-control__item {

// Creates a faux divider between the menu options if present
&.is-create-empty-option {
position: relative; // positioning context for pseudo
Expand Down Expand Up @@ -195,7 +206,6 @@ $color-control-label-height: 20px;
font-family: $default-font;
font-size: $default-font-size;
}

}

.wp-block-navigation .block-editor-button-block-appender {
Expand Down