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: Compute caret DOMRect on-demand for vertical traversal #15607

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link
Member

@aduth aduth commented May 13, 2019

Fixes #15604
Regression of #13697

This pull request seeks to resolve an issue where, when vertically navigating between paragraphs using ArrowUp, ArrowDown, the caret does not always land where would be expected to preserve its horizontal position.

Implementation notes:

See exhaustive debugging detail at #15604 (comment).

Testing instructions:

Repeat Steps to Reproduce from #15604, verifying expected result.

Ensure end-to-end tests pass:

npm run build && npm run test-e2e packages/e2e-tests/specs/writing-flow.test.js

Verify ArrowUp, ArrowDown vertical traversal works generally well in a variety of circumstances.

@aduth aduth added [Type] Bug An existing feature does not function as intended [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... labels May 13, 2019
@aduth aduth added the [Type] Regression Related to a regression in the latest release label May 13, 2019
@aduth
Copy link
Member Author

aduth commented May 13, 2019

@youknowriad We might want to consider this one for backport (WordPress 5.2.1), as it does appear to afflict WordPress 5.2.0.

@ellatrix
Copy link
Member

Will caret position reset when you click somewhere else? I'll test and review in a bit.

@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 14, 2019
@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 14, 2019
@ellatrix
Copy link
Member

ellatrix commented May 14, 2019

This won't work correctly in the following case, which should probably be added to the e2e tests:

Type "1", press Shift+Enter, press Enter, type "2". Place the caret after "1". Press ArrowDown twice. The caret should appear after "2".

In other words, the initial caret position must be preserved between arrow key presses to the closest position to the initial one can be set. It must be unset after mousedown or arrow left/right. It looks like it is only unset on mousedown. Edit: it is.

@aduth
Copy link
Member Author

aduth commented May 14, 2019

Closing in favor of #15624

@aduth aduth closed this May 14, 2019
@aduth aduth deleted the fix/15604-vertical-nav-h-offset branch May 14, 2019 12:16
@aduth aduth removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Writing Flow: Caret position not always preserved in vertical traversal
3 participants