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

Input Interaction: always reset initial vertical position rect #15624

Merged
merged 4 commits into from
May 14, 2019

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented May 14, 2019

Description

Alternative to #15607.

How has this been tested?

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added the [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... label May 14, 2019
await page.keyboard.type( '2' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.type( 'x' ); // Should be right after "1".
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth Here, your PR #15607 will position the "x" before "1", because the initial position is not remembered.

@ellatrix ellatrix added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Type] Regression Related to a regression in the latest release labels May 14, 2019
@ellatrix ellatrix added this to the 5.7 (Gutenberg) milestone May 14, 2019
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fix the issue for me.

@ellatrix ellatrix force-pushed the fix/15604-vertical-nav-h-offset-alt branch from f11bca3 to fff3aed Compare May 14, 2019 12:09
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd wonder if there's a different basis (e.g. selection changes) we should be using for determining whether to reset this value other than specific user interactions, which seem more error-prone to try to cover (e.g. selection changes by taps, selections or field values changing programmatically).

In the interim, this seems like the most direct fix to the original issue 👍 In some stress testing, I think it may also help resolve some cases where they may have been buggy even in the original implementation (e.g. accounting for Home or End keys in selection changes).

packages/block-editor/src/components/writing-flow/index.js Outdated Show resolved Hide resolved
@youknowriad youknowriad merged commit 9683d00 into master May 14, 2019
@youknowriad youknowriad deleted the fix/15604-vertical-nav-h-offset-alt branch May 14, 2019 13:01
@jorgefilipecosta jorgefilipecosta 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 Sep 13, 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.

4 participants