Adds the ability to use UIManager to check if a node is an ancestor #7876

alvaromb · 2016-06-01T14:05:19Z

Motivation

Sometimes is handy to check if a React node is a descendant of another node or not. For instance, I want to check if the focused TextInput is descendant of an specific ScrollView:

const currentlyFocusedField = TextInput.State.currentlyFocusedField()
UIManager.viewIsAncestorOf(
  currentlyFocusedField,
  this.getInnerViewNode(),
  (isAncestor) => {
    if (isAncestor) {
      console.log('The focused field is a descendant of this ScrollView!')
    }
  }
)

This function uses the same strategy as the measureLayout method to check if one node is an ancestor of other node. As the measureLayout method, this is performed outside the main thread.

By now I've only implemented the iOS version and its tests, but if this function is going to be merged I'll implement the Android version too. I have objc experience but no Java or Android, so I prefer to validate this functionality before jumping into developing the Android part.

ghost · 2016-06-01T14:06:23Z

By analyzing the blame information on this pull request, we identified @alloy and @rigdern to be potential reviewers.

ghost · 2016-06-01T14:08:19Z

@alvaromb updated the pull request.

alvaromb · 2016-06-02T07:51:02Z

Probably I should change the name of the function from viewIsAncestorOf to viewIsDescendantOf, it's more correct.

alloy · 2016-06-02T10:14:42Z

I’m not really the right person to judge the functional changes of this PR, but my one point of feedback is that your motivation doesn’t really outline why you want to be able to do this, which makes it harder to understand if this is the right solution. Your code example also doesn’t illustrate a real-world use-case.

alloy · 2016-06-02T10:16:32Z

React/Modules/RCTUIManager.m

+  }
+  if (!ancestor) {
+    return;
+  }


Maybe combine these into a single if (view && ancestor) { … } ?

Can do that. I've just followed the style of the RCTMeasureLayout function.

That’s totally fair. I’m just providing some general code feedback, following existing styles is definitely a good choice 👍

alloy · 2016-06-02T10:24:40Z

While I left some general code feedback (just to be somewhat useful :)), I’ll stress again that this is just that, I’m not the one to decide if this is going to be accepted.

alvaromb · 2016-06-02T10:45:17Z

Sorry if the example isn't clear. Will try to explain why I need this.

Lets say we have a bunch of screens with TextInputs (something like a signup) that are contained by ScrollViews. In order to perform an automatic scroll to the focused TextInput, those ScrollViews can listen to the keyboard events. Once the keyboardWillShow event has been triggered, I can change the scroll contentInset and I can query which TextInput has the focus with the following function:

TextInput.State.currentlyFocusedField()

This will return a tag number which represents the node of the input focused. With ScrollView's scrollResponderScrollNativeHandleToKeyboard, I can use that tag to position the scroll view in order to display the focused input if it's hidden behind the keyboard.

However, the problem arises when you have more than one ScrollView listening to the keyboards at the same time. It might seem unusual, but it's actually quite easy to be in that case. We currently have this combination of scroll views and inputs into the navigation of all of our apps. First screen user inserts some data, then taps a button and navigates to another screen which has another scroll view and input. It's inside the Navigator stack, so both ScrollViews exist and both will receive the keyboard events. In this case, the ScrollView that do not contain the focused input as a descendant will try to scroll to it and crash (see https://github.com/APSL/react-native/blob/master/React/Modules/RCTUIManager.m#L1237).

The solution I came for this specific case was to export a functionality that, in the end, is already present in the measureLayoutRelativeToAncestor function. With this ancestor check I can query if the TextInput node is descendant of the ScrollView and then decide if I can scroll to that input or not.

I hope to have shed some light into the motivations of the PR, and thanks @alloy for your suggestions.

wootwoot1234 · 2016-06-19T22:32:31Z

Any updates on this?

alvaromb · 2016-06-20T07:51:41Z

I'll find some time today to perform the pending changes.

ghost · 2016-06-20T19:26:00Z

Wow, just came to that issue, and the solution is already on the way! 👍

alvaromb · 2016-06-21T08:20:49Z

I've added the detailed suggestions and rebased against master.

grabcode · 2016-06-25T23:45:37Z

Thanks @alvaromb for supporting your great scrollview extension. Could we have some heads up from any guys at FB? @alloy maybe?

- Change method name "viewIsAncestorOf" as "viewIsDescendantOf" according to PR facebook/react-native#7876

wootwoot1234 · 2016-07-08T00:01:07Z

Any update on this?

JosephkimOS · 2016-07-19T17:02:14Z

@alvarom any update on this PR?

alvaromb · 2016-07-20T06:23:19Z

I can update the PR @JosephkimOS, but I don't have any feedback from the Facebook team yet. Let's see if @mkonicek or @vjeux can help us here 😃

irisSchaffer · 2016-08-02T12:58:10Z

Any updates on this?

javache · 2016-08-03T10:13:45Z

@facebook-github-bot shipit

ghost · 2016-08-03T10:14:24Z

Thanks for importing.

Summary: Sometimes is handy to check if a React node is a descendant of another node or not. For instance, I want to check if the focused `TextInput` is descendant of an specific `ScrollView`: ```js const currentlyFocusedField = TextInput.State.currentlyFocusedField() UIManager.viewIsAncestorOf( currentlyFocusedField, this.getInnerViewNode(), (isAncestor) => { if (isAncestor) { console.log('The focused field is a descendant of this ScrollView!') } } ) ``` This function uses the same strategy as the `measureLayout` method to check if one node is an ancestor of other node. As the `measureLayout` method, this is performed outside the main thread. By now I've only implemented the iOS version and its tests, but if this function is going to be merged I'll implement the Android version too. I have objc experience but no Java or Android, so I prefer to validate this functionality before jumping into developing the Android part. Closes facebook#7876 Differential Revision: D3662045 Pulled By: javache fbshipit-source-id: b9668e8ea94fd01db76651f16243926cf9c2566f

wootwoot1234 · 2016-08-03T14:17:42Z

Yeah! Shipit! :)

Thanks for doing this guys. What version or RN will we see this code in?

javache · 2016-08-03T16:16:27Z

This should be in react-native 0.32

JosephDev · 2016-08-03T16:30:12Z

finally!

alvaromb · 2016-08-04T06:41:02Z

Thanks @javache!

I'm going to dig into Android code to send a PR with the same functionality.

Summary: Sometimes is handy to check if a React node is a descendant of another node or not. For instance, I want to check if the focused `TextInput` is descendant of an specific `ScrollView`: ```js const currentlyFocusedField = TextInput.State.currentlyFocusedField() UIManager.viewIsAncestorOf( currentlyFocusedField, this.getInnerViewNode(), (isAncestor) => { if (isAncestor) { console.log('The focused field is a descendant of this ScrollView!') } } ) ``` This function uses the same strategy as the `measureLayout` method to check if one node is an ancestor of other node. As the `measureLayout` method, this is performed outside the main thread. By now I've only implemented the iOS version and its tests, but if this function is going to be merged I'll implement the Android version too. I have objc experience but no Java or Android, so I prefer to validate this functionality before jumping into developing the Android part. Closes facebook/react-native#7876 Differential Revision: D3662045 Pulled By: javache fbshipit-source-id: b9668e8ea94fd01db76651f16243926cf9c2566f

haziqhafizuddin · 2016-08-16T09:33:03Z

TextInput keep crush when goes to next page. i really need this issue to be fix. when is 0.32 roll out!?

ghost · 2016-08-16T09:55:22Z

32-rc is out..

cbcye · 2016-08-18T10:11:37Z

Any updates on this?

alvaromb · 2016-08-18T10:16:58Z

@cbcye that screenshot does not belong to this issue. This PR was accepted and should land into RN 0.32 or 0.33.

Summary: Sometimes is handy to check if a React node is a descendant of another node or not. For instance, I want to check if the focused `TextInput` is descendant of an specific `ScrollView`: ```js const currentlyFocusedField = TextInput.State.currentlyFocusedField() UIManager.viewIsAncestorOf( currentlyFocusedField, this.getInnerViewNode(), (isAncestor) => { if (isAncestor) { console.log('The focused field is a descendant of this ScrollView!') } } ) ``` This function uses the same strategy as the `measureLayout` method to check if one node is an ancestor of other node. As the `measureLayout` method, this is performed outside the main thread. By now I've only implemented the iOS version and its tests, but if this function is going to be merged I'll implement the Android version too. I have objc experience but no Java or Android, so I prefer to validate this functionality before jumping into developing the Android part. Closes facebook#7876 Differential Revision: D3662045 Pulled By: javache fbshipit-source-id: b9668e8ea94fd01db76651f16243926cf9c2566f

cbcye · 2016-10-02T04:39:34Z

@alvaromb i upgrade to RN 0.34 but still occur this problem.

alvaromb · 2016-10-04T08:11:40Z

@cbcye remove the node_modules/ folder and run npm i again. This could be your issue.

Summary: Add the ability for UIManager to check if a node is an ancestor of anther one on Android like #7876 did on iOS Closes #13129 Differential Revision: D4938319 Pulled By: hramos fbshipit-source-id: abe20779be2142a1ea9ac46f52d8cd8609236419

atlas1119 · 2017-06-06T08:08:35Z

i have react native .44 , but still occur this problem.

alvaromb · 2017-06-06T09:29:30Z

@atlas1119 I think that this hasn't landed in Android yet #13129

ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 1, 2016

alloy reviewed Jun 2, 2016
View reviewed changes

alvaromb mentioned this pull request Jun 13, 2016

RCTTextField is not a descendant of RCTShadowView APSL/react-native-keyboard-aware-scroll-view#33

Closed

alvaromb force-pushed the ancestor_of branch from 15a587a to afb0f86 Compare June 21, 2016 08:20

alvaromb mentioned this pull request Jun 27, 2016

Cannot get it to work APSL/react-native-keyboard-aware-scroll-view#24

Closed

JosephDev pushed a commit to JosephDev/react-native-keyboard-aware-scroll-view that referenced this pull request Jun 28, 2016

v0.1.2 with ancestor_check

da27a8c

- Change method name "viewIsAncestorOf" as "viewIsDescendantOf" according to PR facebook/react-native#7876

This was referenced Jun 28, 2016

Ancestor check APSL/react-native-keyboard-aware-scroll-view#41

Closed

'RCTTextField not a descendant of RCTShadowView' crash when TextInput is focused aksonov/react-native-router-flux#865

Closed

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 12, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 19, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 20, 2016

bunea mentioned this pull request Jul 20, 2016

Action type replace has no animation aksonov/react-native-router-flux#939

Closed

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 1, 2016

ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 3, 2016

ghost closed this in e52cab5 Aug 3, 2016

alvaromb mentioned this pull request Aug 11, 2016

Crash when two KASV are rendered and one TextInput gets focused APSL/react-native-keyboard-aware-scroll-view#25

Closed

alvaromb mentioned this pull request Mar 15, 2017

Neither extraScrollHeight nor extraHeight are not working on Android (iOS works) APSL/react-native-keyboard-aware-scroll-view#107

Closed

Swordsman-Inaction mentioned this pull request Mar 24, 2017

Add viewIsDescendantOf for UIManager on Android #13129

Closed

shigggster mentioned this pull request Nov 21, 2018

Avoid multi-line update() app crash by checking first if the focused TextField is a descendant of the ScrollView APSL/react-native-keyboard-aware-scroll-view#322

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds the ability to use UIManager to check if a node is an ancestor #7876

Adds the ability to use UIManager to check if a node is an ancestor #7876

alvaromb commented Jun 1, 2016

ghost commented Jun 1, 2016

ghost commented Jun 1, 2016

alvaromb commented Jun 2, 2016 •

edited

Loading

alloy commented Jun 2, 2016

alloy Jun 2, 2016

alvaromb Jun 2, 2016

alloy Jun 2, 2016

alloy commented Jun 2, 2016

alvaromb commented Jun 2, 2016 •

edited

Loading

wootwoot1234 commented Jun 19, 2016

alvaromb commented Jun 20, 2016

ghost commented Jun 20, 2016

alvaromb commented Jun 21, 2016

grabcode commented Jun 25, 2016

wootwoot1234 commented Jul 8, 2016

JosephkimOS commented Jul 19, 2016

alvaromb commented Jul 20, 2016

irisSchaffer commented Aug 2, 2016

javache commented Aug 3, 2016

ghost commented Aug 3, 2016

wootwoot1234 commented Aug 3, 2016

javache commented Aug 3, 2016

JosephDev commented Aug 3, 2016

alvaromb commented Aug 4, 2016

haziqhafizuddin commented Aug 16, 2016

ghost commented Aug 16, 2016

cbcye commented Aug 18, 2016

alvaromb commented Aug 18, 2016

cbcye commented Oct 2, 2016

alvaromb commented Oct 4, 2016

atlas1119 commented Jun 6, 2017

alvaromb commented Jun 6, 2017

Adds the ability to use UIManager to check if a node is an ancestor #7876

Adds the ability to use UIManager to check if a node is an ancestor #7876

Conversation

alvaromb commented Jun 1, 2016

Motivation

ghost commented Jun 1, 2016

ghost commented Jun 1, 2016

alvaromb commented Jun 2, 2016 • edited Loading

alloy commented Jun 2, 2016

alloy Jun 2, 2016

Choose a reason for hiding this comment

alvaromb Jun 2, 2016

Choose a reason for hiding this comment

alloy Jun 2, 2016

Choose a reason for hiding this comment

alloy commented Jun 2, 2016

alvaromb commented Jun 2, 2016 • edited Loading

wootwoot1234 commented Jun 19, 2016

alvaromb commented Jun 20, 2016

ghost commented Jun 20, 2016

alvaromb commented Jun 21, 2016

grabcode commented Jun 25, 2016

wootwoot1234 commented Jul 8, 2016

JosephkimOS commented Jul 19, 2016

alvaromb commented Jul 20, 2016

irisSchaffer commented Aug 2, 2016

javache commented Aug 3, 2016

ghost commented Aug 3, 2016

wootwoot1234 commented Aug 3, 2016

javache commented Aug 3, 2016

JosephDev commented Aug 3, 2016

alvaromb commented Aug 4, 2016

haziqhafizuddin commented Aug 16, 2016

ghost commented Aug 16, 2016

cbcye commented Aug 18, 2016

alvaromb commented Aug 18, 2016

cbcye commented Oct 2, 2016

alvaromb commented Oct 4, 2016

atlas1119 commented Jun 6, 2017

alvaromb commented Jun 6, 2017

alvaromb commented Jun 2, 2016 •

edited

Loading

alvaromb commented Jun 2, 2016 •

edited

Loading