Skip to content
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

[ScrollView] Consistent .measure API for ScrollView #1060

Closed
JohnyDays opened this issue Apr 29, 2015 · 10 comments
Closed

[ScrollView] Consistent .measure API for ScrollView #1060

JohnyDays opened this issue Apr 29, 2015 · 10 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@JohnyDays
Copy link
Contributor

I would like to allow for any renderable view to be passed in as a component type used in a module, right now the only problem with this approach is that .measure is not present in ScrollView.

I would expect .measure on the ScrollView component to redirect itself to its container, since that is just an implementation detail of the ScrollView.

My current solution is to allow for a function: reachContainer(element: view) prop that gets called and returns the actual measurable container, for this specific case, which is obviously worse than just having a consistent api.

Is .measure just an implementation detail that I shouldn't be using, or is it the defacto api for getting layout details after rendering, if so i'd argue it should be consistent.

@JohnyDays JohnyDays changed the title Consistent .measure API for scroll view Consistent .measure API for ScrollView Apr 29, 2015
@JohnyDays
Copy link
Contributor Author

This appears to be even more complicated than I realized, custom components also do not have a .measure API, nor do they have a standard way of reaching the native view, so as to grab their measures. Am I missing some way to do this? I inspected the element through the dev tools and I could not find how this would be done in a standard way, and I do not want people to have to implement a custom getNativeView interface just to use their components as children in mine.

@vjeux
Copy link
Contributor

vjeux commented Apr 29, 2015

@JohnyDays you can add a ref to the element you render and call measure on this. https://facebook.github.io/react/docs/more-about-refs.html

@JohnyDays
Copy link
Contributor Author

Actually the element does not have a .measure function, I am already using refs.
A snippet of the code I am using as a temporary solution:

layouts.push(new Promise((resolve, reject) => {

  if(!view.getNodeHandle){
    if(!view.getNativeView){
      throw new Error("Non-native views must implement getNativeView")
    }
    var handle = view.getNativeView().getNodeHandle()
  }
  else{
    var handle = view.getNodeHandle()
  }

  RCTUIManager.measure(handle, (x, y, width, height, pageX, pageY) => {resolve({key, value:{x, y, width, height, pageX, pageY}})})
}))

In the above code, the variable "view" contains an element obtained through a ref
Edit: I forgot to mention, I am using this workaround because "view" may not have a .measure function obviously(if it is a scroll view or a custom component), nor is it guaranteed to have .getNodeHandle (as is the case with custom components)

@JohnyDays
Copy link
Contributor Author

@vjeux From your comment, am I correct in assuming any rendered component grabbed from a ref should have a .measure function? If so, maybe this should be reported as a bug, because none of my custom components do, and neither does the ScrollView native component (I am using ES6 classes, maybe this is related?)

@vjeux
Copy link
Contributor

vjeux commented May 1, 2015

Your reading of my comment is correct but i was actually wrong. We need the equivalent of React.findDOMNode() for React Native

@JohnyDays
Copy link
Contributor Author

So in order to implement this, I would assume you would save a unique integer representation of every rendered component, and then asynchronously grab it from some kind of manager in Obj-C, and serialize it through the bridge, correct? e.g

React.findComponent(int: handle, callback: callback)

Given the performance cost this kind of communication/serializing has, I have a few proposals, tell me if I should open a new issue for these.

I think there should be either a React.findComponents(array: ids, callback) or React.findComponents(...ids, callback), so we would only have to pay the cost once.

An even better option, albeit I don't know how easy this would be to implement in obj-c, would be one where no actual serializing of components occurs through the bridge, so as to not allocate unnecessary memory and waste cpu resources, my proposed solution would be the following:

React.callNativeMethodOnComponent(int: id, string: methodName, callback(result): callback)

and an equivalent to the above

React.callNativeMethodOnComponents(array: ids, string: methodName, callback(result): callback)

This would completely remove any costs of serializing components through the bridge, since we only really care about the result of one call to a component method in some cases. An example of this would be for layout purposes, where we only care about the result of .measure() (Whatever this method is called natively) and not about the rest of the component.

Thoughts? Also, what can I do to help?

@brentvatne
Copy link
Collaborator

ping @vjeux

@brentvatne brentvatne changed the title Consistent .measure API for ScrollView [ScrollView] Consistent .measure API for ScrollView May 30, 2015
@JohnyDays
Copy link
Contributor Author

I should close this, since the latest version of react this has been implemented. Thanks @vjeux!

@jonslenkooyala
Copy link

@JohnyDays might you know, where's the commit(s) that address this? I'd love that feature (specifically, for custom components) :-)

@JohnyDays
Copy link
Contributor Author

Not sure what the specific commit is, but It's in these release notes https://github.com/facebook/react-native/releases/tag/v0.4.3 (the new api is React.findNodeHandle(ref))

@facebook facebook locked as resolved and limited conversation to collaborators May 31, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

5 participants