-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Support Navigation and Edit Mode #16500
Merged
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
69dc40d
Support Navigation and Edit Mode
youknowriad 56c1f4e
Support removing blocks from navigation mode
youknowriad df415d5
Add is-navigation className
youknowriad fc8510a
Style adjustments
kjellr fe42bc2
Update API docs
youknowriad e3db2a5
Fix e2e tests by moving to edit mode
youknowriad 1af2e64
alt+f10 only works on edit mode
youknowriad 494bec0
Use blue highlight color all around.
kjellr 58e51f9
no hover styles and breadcrumbs on hover in navigation mode
youknowriad 836a6e5
Fix image navigation mode
youknowriad 6662db5
Trigger the edit mode on mouse down instead of move
youknowriad c9469e6
Fix the edit mode trigger in e2e tests
youknowriad e11c08d
Correct breadcrumb size and text position.
kjellr 680c254
Rename the is-navigate-mode className and fix the docs
youknowriad 1ee2661
Fix typo
youknowriad 439c30f
Better param documentation
youknowriad 1903f3d
Fix typo
youknowriad 21cb1ad
Fix typo
youknowriad 4eb1cf2
Simplify the arrow navigation code
youknowriad 3af2e23
Fix ESCAPE behavior
youknowriad ad94bd7
Fix e2e test edit mode utility
youknowriad fcae4c1
Simplify applied classname
youknowriad 4de1964
Update packages/e2e-test-utils/src/keyboard-mode.js
youknowriad 226a8c8
Use a boolean instead of a string for the keyboard mode
youknowriad 039efc0
Adding comment
youknowriad e75a720
Fix e2e tests
youknowriad 5400dcf
More consistency in the naming of the navigation mode actions/functions
youknowriad aae87af
Limit the edit mode trigger to the writing flow component
youknowriad fbfbe6c
Consistently disable navigation mode in e2e tests
youknowriad File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,13 +8,13 @@ import { animated } from 'react-spring/web.cjs'; | |
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useRef, useEffect, useState } from '@wordpress/element'; | ||
import { useRef, useEffect, useLayoutEffect, useState } from '@wordpress/element'; | ||
import { | ||
focus, | ||
isTextField, | ||
placeCaretAtHorizontalEdge, | ||
} from '@wordpress/dom'; | ||
import { BACKSPACE, DELETE, ENTER } from '@wordpress/keycodes'; | ||
import { BACKSPACE, DELETE, ENTER, ESCAPE } from '@wordpress/keycodes'; | ||
import { | ||
getBlockType, | ||
getSaveElement, | ||
|
@@ -101,6 +101,8 @@ function BlockListBlock( { | |
onSelectionStart, | ||
animateOnChange, | ||
enableAnimation, | ||
isNavigationMode, | ||
enableNavigationMode, | ||
} ) { | ||
// Random state used to rerender the component if needed, ideally we don't need this | ||
const [ , updateRerenderState ] = useState( {} ); | ||
|
@@ -118,6 +120,8 @@ function BlockListBlock( { | |
// Hovered area of the block | ||
const hoverArea = useHoveredArea( wrapper ); | ||
|
||
const breadcrumb = useRef(); | ||
|
||
// Keep track of touchstart to disable hover on iOS | ||
const hadTouchStart = useRef( false ); | ||
const onTouchStart = () => { | ||
|
@@ -215,6 +219,11 @@ function BlockListBlock( { | |
return; | ||
} | ||
|
||
if ( isNavigationMode ) { | ||
breadcrumb.current.focus(); | ||
return; | ||
} | ||
|
||
// Find all tabbables within node. | ||
const textInputs = focus.tabbable | ||
.find( blockNodeRef.current ) | ||
|
@@ -254,6 +263,18 @@ function BlockListBlock( { | |
// Block Reordering animation | ||
const animationStyle = useMovingAnimation( wrapper, isSelected || isPartOfMultiSelection, enableAnimation, animateOnChange ); | ||
|
||
// Focus the breadcrumb if the wrapper is focused on navigation mode. | ||
// Focus the first editable or the wrapper if edit mode. | ||
useLayoutEffect( () => { | ||
if ( isSelected ) { | ||
if ( isNavigationMode ) { | ||
breadcrumb.current.focus(); | ||
} else { | ||
focusTabbable( true ); | ||
} | ||
} | ||
}, [ isSelected, isNavigationMode ] ); | ||
|
||
// Other event handlers | ||
|
||
/** | ||
|
@@ -275,32 +296,43 @@ function BlockListBlock( { | |
* | ||
* @param {KeyboardEvent} event Keydown event. | ||
*/ | ||
const deleteOrInsertAfterWrapper = ( event ) => { | ||
const onKeyDown = ( event ) => { | ||
const { keyCode, target } = event; | ||
|
||
// These block shortcuts should only trigger if the wrapper of the block is selected | ||
// And when it's not a multi-selection to avoid conflicting with RichText/Inputs and multiselection. | ||
if ( | ||
! isSelected || | ||
target !== wrapper.current || | ||
isLocked | ||
) { | ||
return; | ||
} | ||
// ENTER/BACKSPACE Shortcuts are only available if the wrapper is focused | ||
// and the block is not locked. | ||
const canUseShortcuts = ( | ||
isSelected && | ||
! isLocked && | ||
( target === wrapper.current || target === breadcrumb.current ) | ||
); | ||
const isEditMode = ! isNavigationMode; | ||
|
||
switch ( keyCode ) { | ||
case ENTER: | ||
// Insert default block after current block if enter and event | ||
// not already handled by descendant. | ||
onInsertDefaultBlockAfter(); | ||
event.preventDefault(); | ||
if ( canUseShortcuts && isEditMode ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: these nested switch/if statements look a bit odd to me. Could they maybe be combined? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No easy way to do it. We could just use if elses? |
||
// Insert default block after current block if enter and event | ||
// not already handled by descendant. | ||
onInsertDefaultBlockAfter(); | ||
event.preventDefault(); | ||
} | ||
break; | ||
|
||
case BACKSPACE: | ||
case DELETE: | ||
// Remove block on backspace. | ||
onRemove( clientId ); | ||
event.preventDefault(); | ||
if ( canUseShortcuts ) { | ||
// Remove block on backspace. | ||
onRemove( clientId ); | ||
event.preventDefault(); | ||
} | ||
break; | ||
case ESCAPE: | ||
if ( | ||
isSelected && | ||
isEditMode | ||
) { | ||
enableNavigationMode(); | ||
wrapper.current.focus(); | ||
} | ||
break; | ||
} | ||
}; | ||
|
@@ -357,8 +389,8 @@ function BlockListBlock( { | |
|
||
// If the block is selected and we're typing the block should not appear. | ||
// Empty paragraph blocks should always show up as unselected. | ||
const showInserterShortcuts = ( isSelected || isHovered ) && isEmptyDefaultBlock && isValid; | ||
const showEmptyBlockSideInserter = ( isSelected || isHovered || isLast ) && isEmptyDefaultBlock && isValid; | ||
const showInserterShortcuts = ! isNavigationMode && ( isSelected || isHovered ) && isEmptyDefaultBlock && isValid; | ||
const showEmptyBlockSideInserter = ! isNavigationMode && ( isSelected || isHovered || isLast ) && isEmptyDefaultBlock && isValid; | ||
const shouldAppearSelected = | ||
! isFocusMode && | ||
! showEmptyBlockSideInserter && | ||
|
@@ -371,20 +403,23 @@ function BlockListBlock( { | |
! isEmptyDefaultBlock; | ||
// We render block movers and block settings to keep them tabbale even if hidden | ||
const shouldRenderMovers = | ||
! isNavigationMode && | ||
( isSelected || hoverArea === ( isRTL ? 'right' : 'left' ) ) && | ||
! showEmptyBlockSideInserter && | ||
! isPartOfMultiSelection && | ||
! isTypingWithinBlock; | ||
const shouldShowBreadcrumb = | ||
! isFocusMode && isHovered && ! isEmptyDefaultBlock; | ||
( isSelected && isNavigationMode ) || | ||
( ! isNavigationMode && ! isFocusMode && isHovered && ! isEmptyDefaultBlock ); | ||
const shouldShowContextualToolbar = | ||
! isNavigationMode && | ||
! hasFixedToolbar && | ||
! showEmptyBlockSideInserter && | ||
( | ||
( isSelected && ( ! isTypingWithinBlock || isCaretWithinFormattedText ) ) || | ||
isFirstMultiSelected | ||
); | ||
const shouldShowMobileToolbar = shouldAppearSelected; | ||
const shouldShowMobileToolbar = ! isNavigationMode && shouldAppearSelected; | ||
|
||
// Insertion point can only be made visible if the block is at the | ||
// the extent of a multi-selection, or not in a multi-selection. | ||
|
@@ -399,6 +434,7 @@ function BlockListBlock( { | |
{ | ||
'has-warning': ! isValid || !! hasError || isUnregisteredBlock, | ||
'is-selected': shouldAppearSelected, | ||
'is-navigate-mode': isNavigationMode, | ||
'is-multi-selected': isPartOfMultiSelection, | ||
'is-hovered': shouldAppearHovered, | ||
'is-reusable': isReusableBlock( blockType ), | ||
|
@@ -464,7 +500,7 @@ function BlockListBlock( { | |
onTouchStart={ onTouchStart } | ||
onFocus={ onFocus } | ||
onClick={ onTouchStop } | ||
onKeyDown={ deleteOrInsertAfterWrapper } | ||
onKeyDown={ onKeyDown } | ||
tabIndex="0" | ||
aria-label={ blockLabel } | ||
childHandledEvents={ [ 'onDragStart', 'onMouseDown' ] } | ||
|
@@ -509,9 +545,7 @@ function BlockListBlock( { | |
{ shouldShowBreadcrumb && ( | ||
<BlockBreadcrumb | ||
clientId={ clientId } | ||
isHidden={ | ||
! ( isHovered || isSelected ) || hoverArea !== ( isRTL ? 'right' : 'left' ) | ||
} | ||
ref={ breadcrumb } | ||
/> | ||
) } | ||
{ ( shouldShowContextualToolbar || isForcingContextualToolbar.current ) && ( | ||
|
@@ -522,6 +556,7 @@ function BlockListBlock( { | |
/> | ||
) } | ||
{ | ||
! isNavigationMode && | ||
! shouldShowContextualToolbar && | ||
isSelected && | ||
! hasFixedToolbar && | ||
|
@@ -604,6 +639,7 @@ const applyWithSelect = withSelect( | |
getBlockIndex, | ||
getBlockOrder, | ||
__unstableGetBlockWithoutInnerBlocks, | ||
isNavigationMode, | ||
} = select( 'core/block-editor' ); | ||
const block = __unstableGetBlockWithoutInnerBlocks( clientId ); | ||
const isSelected = isBlockSelected( clientId ); | ||
|
@@ -637,6 +673,7 @@ const applyWithSelect = withSelect( | |
isFocusMode: focusMode && isLargeViewport, | ||
hasFixedToolbar: hasFixedToolbar && isLargeViewport, | ||
isLast: index === blockOrder.length - 1, | ||
isNavigationMode: isNavigationMode(), | ||
isRTL, | ||
|
||
// Users of the editor.BlockListBlock filter used to be able to access the block prop | ||
|
@@ -664,6 +701,7 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => { | |
mergeBlocks, | ||
replaceBlocks, | ||
toggleSelection, | ||
setNavigationMode, | ||
} = dispatch( 'core/block-editor' ); | ||
|
||
return { | ||
|
@@ -737,6 +775,9 @@ const applyWithDispatch = withDispatch( ( dispatch, ownProps, { select } ) => { | |
toggleSelection( selectionEnabled ) { | ||
toggleSelection( selectionEnabled ); | ||
}, | ||
enableNavigationMode() { | ||
setNavigationMode( true ); | ||
}, | ||
}; | ||
} ); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason we want to prevent switching modes when the template is locked? Isn't it still useful to be able to navigate content, even if the arrangement of said content can't be modified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That boolean is not used for mode switching. So it should still be possible.