fix: Native inner blocks merge where appropriate #45048
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
An error was thrown when attempting to merge inner blocks via pressing the backward delete key. This applies the remainder of a recent web refactor to avoid errors and crashes. The spirit of the web refactor was to improve the experience when interacting with List V2 blocks.
dcalhoun
added
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
- List blocks merge into other list blocks. - List blocks unwrap list items when merging into non-list blocks.
dcalhoun
force-pushed
the
fix/merge-native-inner-blocks
branch
from
32e9332
to
e9e4526
Compare
dcalhoun
added
the
Mobile App - Automation
derekblank
approved these changes
Oct 19, 2022
|
I noticed that the |
Not for this PR but I'm wondering if we could extract the logic within the |
Solved ✅ after updating the PR with the latest changes from |
|
Hi all, does this fix need to backported to 6.1? |
Hey @ndiego, thanks for the heads up 🙇. AFAIK, this PR only updates files of the native version (i.e. files with extension |
@fluiddot I agree. I considered this as well, and it reinforced my concerns about the diverges of web and native components. For this specific case, merely abstracting
@ndiego thanks for checking! The native editor has its own, separate release cycle. I do not believe the code changes here impact the web editor and therefore do not need to be considered for a specific web editor release. |
|
Ok cool, thanks for the clarification! |
@dcalhoun I totally agree. Ideally, we should only have one This is definitely a challenge as it can be tricky to follow this structure across the Gutenberg codebase. Nevertheless, a huge improvement in getting both versions closer in terms of logic. One part that will be particularly complex to tackle is the rendering since the markup and elements used in each version usually differ. |
Indeed. I feel #18018 is somewhat related to this subject, and opens discussion on how we compose markup/styles.
Yep, an example is already showcasing itself where #45075, a continuation of this PR's origin, should be applied to native as well. |
Good point, let's follow up on that PR to include it in the native version once it's merged 👍 . |
fluiddot
added a commit
that referenced
this pull request
Oct 19, 2022
* fix: Native inner blocks merge where appropriate An error was thrown when attempting to merge inner blocks via pressing the backward delete key. This applies the remainder of a recent web refactor to avoid errors and crashes. The spirit of the web refactor was to improve the experience when interacting with List V2 blocks. * test: Add List block merge test - List blocks merge into other list blocks. - List blocks unwrap list items when merging into non-list blocks. Co-authored-by: Carlos Garcia <[email protected]>
4 tasks
derekblank
pushed a commit
that referenced
this pull request
Oct 20, 2022
* Release script: Update react-native-editor version to 1.84.0 * Release script: Update with changes from 'npm run core preios' * Update react-native-editor changelog * Release script: Update react-native-editor version to 1.84.1 * Release script: Update with changes from 'npm run core preios' * fix: Native inner blocks merge where appropriate (#45048) * fix: Native inner blocks merge where appropriate An error was thrown when attempting to merge inner blocks via pressing the backward delete key. This applies the remainder of a recent web refactor to avoid errors and crashes. The spirit of the web refactor was to improve the experience when interacting with List V2 blocks. * test: Add List block merge test - List blocks merge into other list blocks. - List blocks unwrap list items when merging into non-list blocks. Co-authored-by: Carlos Garcia <[email protected]> * Update react-native-editor changelog Co-authored-by: David Calhoun <[email protected]>
dcalhoun
added a commit
to wordpress-mobile/test-cases
that referenced
this pull request
Jul 6, 2023
This was automated in WordPress/gutenberg#45048.
dcalhoun
added a commit
to wordpress-mobile/test-cases
that referenced
this pull request
Jul 6, 2023
This was automated in WordPress/gutenberg#45048.
dcalhoun
added a commit
to wordpress-mobile/test-cases
that referenced
this pull request
Jul 6, 2023
This was automated in WordPress/gutenberg#45048.
dcalhoun
added a commit
to wordpress-mobile/test-cases
that referenced
this pull request
Jul 10, 2023
* docs: Remove duplicative Dark Mode Group block test case This test case is duplicative of the Group test case: https://github.com/wordpress-mobile/test-cases/blob/2389290ec634b25dcd19d1d2af3badcf8591ff4a/test-cases/gutenberg/group.md#tc009 * docs: Remove automated Rich Text cases These were automated in WordPress/gutenberg#49352. * docs: Remove automated List splitting and merging test This was automated in WordPress/gutenberg#45048. * docs: Remove automated undo and redo test cases These were automated in WordPress/gutenberg#49352.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Apply #43181, a recent web-based refactor for List merging, to the native components to avoid errors and crashes. The spirit of the web refactor was to improve the experience when interacting with List V2 blocks.
Why?
Fixes #44990. Allows merging Lists into other Lists and unwrapping Lists when merging into non-Lists.
How?
Apply the changes from #43181 to the sibling
packages/block-editor/src/components/block-list/block.native.jsfile. Additionally, pass the requiredonMergeandrootClienIdprops down the tree to the relevant locations.Testing Instructions
Sample Markup
From my research, mobile OS' do not support forward delete, i.e. the Delete key rather than Backspace key. So, not every test criteria from #43181 is applicable, and therefore are struck below.
Place the caret at the end of the first list and press Delete. Should merge the lists. Undo.Place the caret at the end of the second list and press Delete. Should merge the second paragraph with the list. Undo.Screenshots or screencast
Screen.Recording.2022-10-18.at.20.37.38.mov