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

Writing flow: fix caret movement for multiple lines #42423

Merged
merged 2 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion packages/dom/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ _Parameters_

_Returns_

- `DOMRect`: The rectangle.
- `DOMRect?`: The rectangle.
ellatrix marked this conversation as resolved.
Show resolved Hide resolved

### getScrollContainer

Expand Down
12 changes: 10 additions & 2 deletions packages/dom/src/dom/get-rectangle-from-range.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { assertIsDefined } from '../utils/assert-is-defined';
*
* @param {Range} range The range.
*
* @return {DOMRect} The rectangle.
* @return {DOMRect?} The rectangle.
*/
export default function getRectangleFromRange( range ) {
// For uncollapsed ranges, get the rectangle that bounds the contents of the
Expand Down Expand Up @@ -73,7 +73,15 @@ export default function getRectangleFromRange( range ) {
range.setEnd( parentNode, index );
}

let rect = range.getClientRects()[ 0 ];
const rects = range.getClientRects();

// If we have multiple rectangles for a collapsed range, there's no way to
// know which it is, so don't return anything.
if ( rects.length > 1 ) {
return null;
}

let rect = rects[ 0 ];

// If the collapsed range starts (and therefore ends) at an element node,
// `getClientRects` can be empty in some browsers. This can be resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ exports[`Writing Flow should merge paragraphs 1`] = `
<!-- /wp:paragraph -->"
`;

exports[`Writing Flow should move to the start of the first line on ArrowUp 1`] = `
"<!-- wp:paragraph -->
<p>.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa</p>
<!-- /wp:paragraph -->"
`;

exports[`Writing Flow should navigate around inline boundaries 1`] = `
"<!-- wp:paragraph -->
<p>FirstAfter</p>
Expand Down
28 changes: 28 additions & 0 deletions packages/e2e-tests/specs/editor/various/writing-flow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -753,4 +753,32 @@ describe( 'Writing Flow', () => {
);
expect( paragraphs ).toEqual( [ 'first', 'second' ] );
} );

it( 'should move to the start of the first line on ArrowUp', async () => {
await page.keyboard.press( 'Enter' );
await page.keyboard.type( 'a' );

async function getHeight() {
return await page.evaluate(
() => document.activeElement.offsetHeight
);
}

const height = await getHeight();

// Keep typing until the height of the element increases. We need two
// lines.
while ( height === ( await getHeight() ) ) {
await page.keyboard.type( 'a' );
}

// Move to the start of the second line.
await page.keyboard.press( 'ArrowLeft' );
// Move to the start of the first line.
await page.keyboard.press( 'ArrowUp' );
// Insert a "." for testing.
await page.keyboard.type( '.' );
// Expect the "." to be at the start of the paragraph.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );