Skip to content

Commit

Permalink
Writing flow: fix vertical arrow keys not moving (#53454)
Browse files Browse the repository at this point in the history
  • Loading branch information
ellatrix authored Aug 9, 2023
1 parent a3bef32 commit 4a24e85
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 33 deletions.
4 changes: 2 additions & 2 deletions packages/dom/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ Check whether the selection is horizontally at the edge of the container.

_Parameters_

- _container_ `Element`: Focusable element.
- _container_ `HTMLElement`: Focusable element.
- _isReverse_ `boolean`: Set to true to check left, false for right.

_Returns_
Expand Down Expand Up @@ -269,7 +269,7 @@ Check whether the selection is vertically at the edge of the container.

_Parameters_

- _container_ `Element`: Focusable element.
- _container_ `HTMLElement`: Focusable element.
- _isReverse_ `boolean`: Set to true to check top, false for bottom.

_Returns_
Expand Down
14 changes: 6 additions & 8 deletions packages/dom/src/dom/is-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@ import isSelectionForward from './is-selection-forward';
import hiddenCaretRangeFromPoint from './hidden-caret-range-from-point';
import { assertIsDefined } from '../utils/assert-is-defined';
import isInputOrTextArea from './is-input-or-text-area';
import { scrollIfNoRange } from './scroll-if-no-range';

/**
* Check whether the selection is at the edge of the container. Checks for
* horizontal position by default. Set `onlyVertical` to true to check only
* vertically.
*
* @param {Element} container Focusable element.
* @param {boolean} isReverse Set to true to check left, false to check right.
* @param {boolean} [onlyVertical=false] Set to true to check only vertical position.
* @param {HTMLElement} container Focusable element.
* @param {boolean} isReverse Set to true to check left, false to check right.
* @param {boolean} [onlyVertical=false] Set to true to check only vertical position.
*
* @return {boolean} True if at the edge, false if not.
*/
Expand Down Expand Up @@ -96,11 +97,8 @@ export default function isEdge( container, isReverse, onlyVertical = false ) {
// pixels. `getComputedStyle` may return a value with different units.
const x = isReverseDir ? containerRect.left + 1 : containerRect.right - 1;
const y = isReverse ? containerRect.top + 1 : containerRect.bottom - 1;
const testRange = hiddenCaretRangeFromPoint(
ownerDocument,
x,
y,
/** @type {HTMLElement} */ ( container )
const testRange = scrollIfNoRange( container, isReverse, () =>
hiddenCaretRangeFromPoint( ownerDocument, x, y, container )
);

if ( ! testRange ) {
Expand Down
4 changes: 2 additions & 2 deletions packages/dom/src/dom/is-horizontal-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import isEdge from './is-edge';
/**
* 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 {HTMLElement} 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.
*/
Expand Down
4 changes: 2 additions & 2 deletions packages/dom/src/dom/is-vertical-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import isEdge from './is-edge';
/**
* 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 {HTMLElement} container Focusable element.
* @param {boolean} isReverse Set to true to check top, false for bottom.
*
* @return {boolean} True if at the vertical edge, false if not.
*/
Expand Down
24 changes: 5 additions & 19 deletions packages/dom/src/dom/place-caret-at-edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import hiddenCaretRangeFromPoint from './hidden-caret-range-from-point';
import { assertIsDefined } from '../utils/assert-is-defined';
import isInputOrTextArea from './is-input-or-text-area';
import isRTL from './is-rtl';
import { scrollIfNoRange } from './scroll-if-no-range';

/**
* Gets the range to place.
Expand Down Expand Up @@ -70,26 +71,11 @@ export default function placeCaretAtEdge( container, isReverse, x ) {
return;
}

let range = getRange( container, isReverse, x );
const range = scrollIfNoRange( container, isReverse, () =>
getRange( container, isReverse, x )
);

// If no range range can be created or it is outside the container, the
// element may be out of view.
if (
! range ||
! range.startContainer ||
! container.contains( range.startContainer )
) {
container.scrollIntoView( isReverse );
range = range = getRange( container, isReverse, x );

if (
! range ||
! range.startContainer ||
! container.contains( range.startContainer )
) {
return;
}
}
if ( ! range ) return;

const { ownerDocument } = container;
const { defaultView } = ownerDocument;
Expand Down
34 changes: 34 additions & 0 deletions packages/dom/src/dom/scroll-if-no-range.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* If no range range can be created or it is outside the container, the element
* may be out of view, so scroll it into view and try again.
*
* @param {HTMLElement} container The container to scroll.
* @param {boolean} alignToTop True to align to top, false to bottom.
* @param {Function} callback The callback to create the range.
*
* @return {?Range} The range returned by the callback.
*/
export function scrollIfNoRange( container, alignToTop, callback ) {
let range = callback();

// If no range range can be created or it is outside the container, the
// element may be out of view.
if (
! range ||
! range.startContainer ||
! container.contains( range.startContainer )
) {
container.scrollIntoView( alignToTop );
range = callback();

if (
! range ||
! range.startContainer ||
! container.contains( range.startContainer )
) {
return null;
}
}

return range;
}

0 comments on commit 4a24e85

Please sign in to comment.