[RNMobile] Narrow down blur on unmount to replaceable only #16151
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
In #15999 this behavior was changed to prevent the keyboard from disappearing when you press Enter to create a new paragraph. This worked in that case, but had the side effect of TextInputState still thinking a dead view had focus, and causing a crash in some scenarios.
That RichText won't have the chance to blur earlier so, we need to do it on unmount. But, non replaceable blocks wont need to blur their RichText on unmount because that will happen normally in componentDidUpdate().
hypest
requested review from
ellatrix,
gziolo,
talldan and
youknowriad
as code owners
hypest
added
the
Mobile App - i.e. Android or iOS
1 task
hypest
changed the base branch from
rnmobile/1126-fix-blur-crash
to
rnmobile/release-v1.7.0
hypest
requested review from
koke
and removed request for
ellatrix,
talldan,
youknowriad and
gziolo
…nmount-to-replaceable
marecar3
approved these changes
Jun 13, 2019
hypest
added a commit
that referenced
this pull request
Jun 14, 2019
* [Mobile] Disable Video block on Android for v1.7.0 release (#16149) * Disable Video block on Android for v1.7.0 release * Add import statement to check platform * [RNMobile] Narrow down blur on unmount to replaceable only (#16151) * Remove focus from RichText when unmounting In #15999 this behavior was changed to prevent the keyboard from disappearing when you press Enter to create a new paragraph. This worked in that case, but had the side effect of TextInputState still thinking a dead view had focus, and causing a crash in some scenarios. * Only blur a RichText in replaceable block That RichText won't have the chance to blur earlier so, we need to do it on unmount. But, non replaceable blocks wont need to blur their RichText on unmount because that will happen normally in componentDidUpdate(). * Rename to an unstable, very goal-named name * Compute shouldBlurOnUnmount inside RichText * Fix no-duplicate-imports lint issue * Fix comma-dangle lint error * Remove trailing space from comment
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
Additional PR on top of #16146, to add more conditions on when to do the blur-on-unmount for the RichText.
In this case, we're passing theInstead of passing it, the latest revision of this fix has the computation of theisReplaceableflag to the component via the Context, and do the blur-on-unmount only if the parent block is replaceable (in which case it won't have the chance the blur otherwise).shouldBlurOnUnmountflag/prop right inside thewithSelectHOC of the RichText.(Embedding the description from #16146 for completeness):
In #15999 this behavior was changed to prevent the keyboard from disappearing
when you press Enter to create a new paragraph.
This worked in that case, but had the side effect of TextInputState still
thinking a dead view had focus, and causing a crash in some scenarios.
Fixes wordpress-mobile/gutenberg-mobile#1126
To test:
Types of changes
Compute and pass
isReplaceableto the editing context and use it in RichText to blur.Checklist: