Fix incorrect block insertion point after blurring post title #32831
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
dcalhoun
added
[Feature] Inserter
Selecting the title selection status within the same `useSelect` for reusable blocks caused the title selection status to become stale, due to the dependency array for the `useSelect` hook. The staleness caused the block insertion point to show up in the incorrect location after blurring the title text input.
|
Size Change: 0 B Total Size: 1.04 MB ℹ️ View Unchanged
|
dcalhoun
force-pushed
the
fix/incorrect-block-insertion-after-title-focus
branch
from
67a64cf to
8f5443d
Compare
dcalhoun
added
the
[Type] Regression
Yep, it can be tricky to assert the order, I'm wondering if we could do something similar to what we do in the UI tests. I saw that we use two methods for this purpose, the first one (used in I think both should work for this case but I'd lean on the second one because we could check also the content of the blocks, wdyt? |
fluiddot
approved these changes
Jun 21, 2021
The aspect that inhibited me asserting the order was that the insertion point does not have any positional information in its text or labels. However, I had not considered asserting that a newly add block is in the correct position. I like this idea because not only would it address the inhibitor I encountered, but it also tests the position of an inserted a block, rather than merely testing the position of the insertion point indicator. I will follow up with a separate PR to add tests asserting against the HTML. Thanks for the great idea! |
Awesome! In case you plan to add the blocks via the inserter, I opened this PR for the Reusable block that includes some fixes I had to do to enable the insertion. The tests pass so feel free to take those changes, but it's not ready yet because I wanted to refactor the |
dcalhoun
added a commit
that referenced
this pull request
Jun 22, 2021
Selecting the title selection status within the same `useSelect` for reusable blocks caused the title selection status to become stale, due to the dependency array for the `useSelect` hook. The staleness caused the block insertion point to show up in the incorrect location after blurring the title text input.
5 tasks
dcalhoun
added a commit
that referenced
this pull request
Jun 23, 2021
* Release script: Update react-native-editor version to 1.55.0 * Release script: Update with changes from 'npm run core preios' * Release script: Update react-native-editor version to 1.55.1 * Release script: Update with changes from 'npm run core preios' * Fix: RNMobile borderRadius value setting (#32717) * [RNMobile] Improve unsupported block message for reusable block (#32618) * Improve unsupported block message for reusable block The unsupported block message has been updated and the convert to regular blocks button has been also included. * Update unsupported block message of reusable block * Update react-native-editor changelog * Release script: Update react-native-editor version to 1.55.2 * Release script: Update with changes from 'npm run core preios' * Fix incorrect block insertion point after blurring post title (#32831) Selecting the title selection status within the same `useSelect` for reusable blocks caused the title selection status to become stale, due to the dependency array for the `useSelect` hook. The staleness caused the block insertion point to show up in the incorrect location after blurring the title text input. * Update changelog * Properly handle 404 errors while publishing Android artifacts (#32860) Co-authored-by: Antonis Lilis <[email protected]> Co-authored-by: Carlos Garcia <[email protected]> Co-authored-by: Enej Bajgoric <[email protected]> Co-authored-by: Oguz Kocer <[email protected]>
dcalhoun
added a commit
that referenced
this pull request
Jun 23, 2021
Add tests for a recently fix regression to decrease the changes of it from occurring again. Relates to #32831.
7 tasks
7 tasks
dcalhoun
added a commit
that referenced
this pull request
Feb 4, 2022
Relates to #32831. Adds tests asserting that block inserter inserts new blocks in the correct position after blurring the post title field.
fluiddot
added a commit
that referenced
this pull request
May 10, 2022
Relates to #32831. Adds tests asserting that block inserter inserts new blocks in the correct position after blurring the post title field. Co-authored-by: Carlos Garcia <[email protected]>
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.
Description
Fixes #32154. Selecting the title selection status within the same
useSelectfor reusable blocks caused the title selection status to become stale, due to the dependency array for theuseSelecthook. The staleness caused the block insertion point to show up in the incorrect location after blurring the title text input.How has this been tested?
I attempted to add automated tests for this but ran into issues asserting the order of the elements. The primary issue was that our current tree structure does not have queryable attributes on the relevant elements, with no easy fix obvious to me. Additionally, React Native Testing Library's lack of support for traversing upwards the tree or snapshotting a subset of a rendered tree inhibited alternative methods.
The following was manually tested:
Screenshots
Before
block-insertion-before.mov
After
block-insertion-after.mov
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).