-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Writing Flow: Fix vertical arrow navigation skips #7877
Merged
Merged
Changes from all commits
Commits
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 |
---|---|---|
@@ -1,25 +1,73 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { includes, first } from 'lodash'; | ||
import tinymce from 'tinymce'; | ||
import { includes } from 'lodash'; | ||
|
||
/** | ||
* Browser dependencies | ||
*/ | ||
const { getComputedStyle, DOMRect } = window; | ||
const { TEXT_NODE, ELEMENT_NODE } = window.Node; | ||
|
||
const { getComputedStyle } = window; | ||
const { | ||
TEXT_NODE, | ||
ELEMENT_NODE, | ||
DOCUMENT_POSITION_PRECEDING, | ||
DOCUMENT_POSITION_FOLLOWING, | ||
} = window.Node; | ||
|
||
/** | ||
* Returns true if the given selection object is in the forward direction, or | ||
* false otherwise. | ||
* | ||
* @see https://developer.mozilla.org/en-US/docs/Web/API/Node/compareDocumentPosition | ||
* | ||
* @param {Selection} selection Selection object to check. | ||
* | ||
* @return {boolean} Whether the selection is forward. | ||
*/ | ||
function isSelectionForward( selection ) { | ||
const { | ||
anchorNode, | ||
focusNode, | ||
anchorOffset, | ||
focusOffset, | ||
} = selection; | ||
|
||
const position = anchorNode.compareDocumentPosition( focusNode ); | ||
|
||
// Disable reason: `Node#compareDocumentPosition` returns a bitmask value, | ||
// so bitwise operators are intended. | ||
/* eslint-disable no-bitwise */ | ||
// Compare whether anchor node precedes focus node. If focus node (where | ||
// end of selection occurs) is after the anchor node, it is forward. | ||
if ( position & DOCUMENT_POSITION_PRECEDING ) { | ||
return false; | ||
} | ||
|
||
if ( position & DOCUMENT_POSITION_FOLLOWING ) { | ||
return true; | ||
} | ||
/* eslint-enable no-bitwise */ | ||
|
||
// `compareDocumentPosition` returns 0 when passed the same node, in which | ||
// case compare offsets. | ||
if ( position === 0 ) { | ||
return anchorOffset <= focusOffset; | ||
} | ||
|
||
// This should never be reached, but return true as default case. | ||
return true; | ||
} | ||
|
||
/** | ||
* Check whether the caret is horizontally at the edge of the container. | ||
* Check whether the selection is horizontally at the edge of the container. | ||
* | ||
* @param {Element} container Focusable element. | ||
* @param {boolean} isReverse Set to true to check left, false for right. | ||
* @param {boolean} collapseRanges Whether or not to collapse the selection range before the check. | ||
* @param {Element} container Focusable element. | ||
* @param {boolean} isReverse Set to true to check left, false for right. | ||
* | ||
* @return {boolean} True if at the horizontal edge, false if not. | ||
*/ | ||
export function isHorizontalEdge( container, isReverse, collapseRanges = false ) { | ||
export function isHorizontalEdge( container, isReverse ) { | ||
if ( includes( [ 'INPUT', 'TEXTAREA' ], container.tagName ) ) { | ||
if ( container.selectionStart !== container.selectionEnd ) { | ||
return false; | ||
|
@@ -36,61 +84,36 @@ export function isHorizontalEdge( container, isReverse, collapseRanges = false ) | |
return true; | ||
} | ||
|
||
// If the container is empty, the caret is always at the edge. | ||
if ( tinymce.DOM.isEmpty( container ) ) { | ||
return true; | ||
} | ||
|
||
const selection = window.getSelection(); | ||
let range = selection.rangeCount ? selection.getRangeAt( 0 ) : null; | ||
if ( collapseRanges ) { | ||
range = range.cloneRange(); | ||
range.collapse( isReverse ); | ||
} | ||
|
||
if ( ! range || ! range.collapsed ) { | ||
return false; | ||
} | ||
|
||
const position = isReverse ? 'start' : 'end'; | ||
const order = isReverse ? 'first' : 'last'; | ||
const offset = range[ `${ position }Offset` ]; | ||
|
||
let node = range.startContainer; | ||
|
||
if ( isReverse && offset !== 0 ) { | ||
return false; | ||
} | ||
|
||
const maxOffset = node.nodeType === TEXT_NODE ? node.nodeValue.length : node.childNodes.length; | ||
// Create copy of range for setting selection to find effective offset. | ||
const range = selection.getRangeAt( 0 ).cloneRange(); | ||
|
||
if ( ! isReverse && offset !== maxOffset ) { | ||
return false; | ||
// Collapse in direction of selection. | ||
if ( ! selection.isCollapsed ) { | ||
range.collapse( ! isSelectionForward( selection ) ); | ||
} | ||
|
||
while ( node !== container ) { | ||
const parentNode = node.parentNode; | ||
|
||
if ( parentNode[ `${ order }Child` ] !== node ) { | ||
return false; | ||
} | ||
|
||
node = parentNode; | ||
} | ||
const { endContainer, endOffset } = range; | ||
range.selectNodeContents( container ); | ||
range.setEnd( endContainer, endOffset ); | ||
|
||
return true; | ||
// Check if the caret position is at the expected start/end position based | ||
// on the value of `isReverse`. If so, consider the horizontal edge to be | ||
// reached. | ||
const caretOffset = range.toString().length; | ||
return caretOffset === ( isReverse ? 0 : container.textContent.length ); | ||
} | ||
|
||
/** | ||
* Check whether the caret is vertically at the edge of the container. | ||
* Check whether the selection is vertically at the edge of the container. | ||
* | ||
* @param {Element} container Focusable element. | ||
* @param {boolean} isReverse Set to true to check top, false for bottom. | ||
* @param {boolean} collapseRanges Whether or not to collapse the selection range before the check. | ||
* @param {Element} container Focusable element. | ||
* @param {boolean} isReverse Set to true to check top, false for bottom. | ||
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. Same here, I just find this API a bit odd. If we have to do deprecations to change it we could file an issue, but I just find it hard to understand in isolation. |
||
* | ||
* @return {boolean} True if at the edge, false if not. | ||
*/ | ||
export function isVerticalEdge( container, isReverse, collapseRanges = false ) { | ||
export function isVerticalEdge( container, isReverse ) { | ||
if ( includes( [ 'INPUT', 'TEXTAREA' ], container.tagName ) ) { | ||
return isHorizontalEdge( container, isReverse ); | ||
} | ||
|
@@ -100,16 +123,8 @@ export function isVerticalEdge( container, isReverse, collapseRanges = false ) { | |
} | ||
|
||
const selection = window.getSelection(); | ||
let range = selection.rangeCount ? selection.getRangeAt( 0 ) : null; | ||
if ( collapseRanges && range && ! range.collapsed ) { | ||
const newRange = document.createRange(); | ||
// Get the end point of the selection (see focusNode vs. anchorNode) | ||
newRange.setStart( selection.focusNode, selection.focusOffset ); | ||
newRange.collapse( true ); | ||
range = newRange; | ||
} | ||
|
||
if ( ! range || ! range.collapsed ) { | ||
const range = selection.rangeCount ? selection.getRangeAt( 0 ) : null; | ||
if ( ! range ) { | ||
return false; | ||
} | ||
|
||
|
@@ -150,21 +165,21 @@ export function getRectangleFromRange( range ) { | |
return range.getBoundingClientRect(); | ||
} | ||
|
||
// If the collapsed range starts (and therefore ends) at an element node, | ||
// `getClientRects` will return undefined. To fix this we can get the | ||
// bounding rectangle of the element node to create a DOMRect based on that. | ||
if ( range.startContainer.nodeType === ELEMENT_NODE ) { | ||
const { x, y, height } = range.startContainer.getBoundingClientRect(); | ||
|
||
// Create a new DOMRect with zero width. | ||
return new DOMRect( x, y, 0, height ); | ||
} | ||
let rect = range.getClientRects()[ 0 ]; | ||
|
||
// For normal collapsed ranges (exception above), the bounding rectangle of | ||
// the range may be inaccurate in some browsers. There will only be one | ||
// rectangle since it is a collapsed range, so it is safe to pass this as | ||
// the union of them. This works consistently in all browsers. | ||
return first( range.getClientRects() ); | ||
// If the collapsed range starts (and therefore ends) at an element node, | ||
// `getClientRects` can be empty in some browsers. This can be resolved | ||
// by adding a temporary text node with zero-width space to the range. | ||
// | ||
// See: https://stackoverflow.com/a/6847328/995445 | ||
if ( ! rect ) { | ||
const padNode = document.createTextNode( '\u200b' ); | ||
range.insertNode( padNode ); | ||
rect = range.getClientRects()[ 0 ]; | ||
padNode.parentNode.removeChild( padNode ); | ||
} | ||
|
||
return rect; | ||
} | ||
|
||
/** | ||
|
@@ -182,7 +197,7 @@ export function computeCaretRect( container ) { | |
const selection = window.getSelection(); | ||
const range = selection.rangeCount ? selection.getRangeAt( 0 ) : null; | ||
|
||
if ( ! range || ! range.collapsed ) { | ||
if ( ! range ) { | ||
return; | ||
} | ||
|
||
|
@@ -314,7 +329,7 @@ export function placeCaretAtVerticalEdge( container, isReverse, rect, mayUseScro | |
// equivalent to a point at half the height of a line of text. | ||
const buffer = rect.height / 2; | ||
const editableRect = container.getBoundingClientRect(); | ||
const x = rect.left + ( rect.width / 2 ); | ||
const x = rect.left; | ||
const y = isReverse ? ( editableRect.bottom - buffer ) : ( editableRect.top + buffer ); | ||
const selection = window.getSelection(); | ||
|
||
|
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
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.
This doesn't work properly when the paragraph block starts with
<br />
Related #8388
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.
Would appreciate your review at #8461
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.
Cool missed that PR :)