-
Notifications
You must be signed in to change notification settings - Fork 4.6k
After Enter transform, skip other onEnter actions like splitting #59064
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
Conversation
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 leads to the onCustomEnter handler not being called, but that should be OK. If I understand correctly, it's used to handle things like list splitting (#43949).
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.
The #43949 test plan succeeds for me on an iPhone 15 Pro simulator.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ellatrix
left a comment
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.
Good catch
|
Size Change: +4 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 6eeb7d9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7931141170
|
6890e78 to
b64c7fc
Compare
dcalhoun
left a comment
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.
It doesn't appear the use case of typing --- is supported on mobile currently, but the changes do not appear to introduce regressions. 🚀
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.
The #43949 test plan succeeds for me on an iPhone 15 Pro simulator.
tyxla
left a comment
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.
Great find, and works like a charm 👍
b64c7fc to
6eeb7d9
Compare
|
There are permanent e2e test failures when testing the Navigation block. It's like if this branch was running the e2e tests with WordPress 6.4, then they would fail as described in https://github.com/WordPress/gutenberg/pull/58389/files#r1492414765. Locally, when I run them against WP 6.5, the tests all succeed. And there is no good reason for the Navigation tests to fail, there is no |
) Co-authored-by: jsnajdr <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: dcalhoun <[email protected]> Co-authored-by: tyxla <[email protected]>
|
I just cherry-picked this PR to the cherry-pick-beta-2 branch to get it included in the next release: 54b7e41 |
) Co-authored-by: jsnajdr <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: dcalhoun <[email protected]> Co-authored-by: tyxla <[email protected]>
If I type
---and then hit Enter, it triggers a block transform that creates the Separator block. This should be the end of it, but currently theuseEnter/onEnterhandlers continue handling the Enter keypress and try to do things like block splitting. But they shouldn't do this. It leads to a bogusREPLACE_BLOCKSaction that tries to do a split, but it does nothing because the blocks to be replaced were already replaced by a previousREPLACE_BLOCKSaction (which created the Separator).How to test:
Put a logpoint on the
REPLACE_BLOCKSaction. Before this patch, there are two of them dispatched on Enter. After this patch, only one is dispatched.