Android shouldn't dispatch onLayout if frame didn't change

[hayeah]

Fixes #7202 Android Redundant onLayout event

[ghost]

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

[hayeah]

ping @mkonicek @korDen

[mkonicek]

nit: space after if, split on multiple lines

[hayeah]

you mean split the conditions on multiple lines?

[mkonicek]

We should probably have a dedicated API to detect screen rotation other than:

<RootView onLayout={onDeviceRotation}>

Does this one fire if keyboard is shown for example?

[mkonicek]

Thanks for the great description in #7202.

[mkonicek]

I pinged some people internally to get more opinions.

[hayeah]

In my own code I use onLayout in a top-level component to receive the frame. Once the container component has the frame it passes the frame down the tree hierarchy, which I use as a ersatz "percentage unit".

Something like this:

<Measure onLayout=(this.renderWithLayout)/>

In the case of keyboard event, this works exactly the same as orientation... It might be nice to have something baked-in to capture the "percentage width" concept.

[ghost]

@hayeah updated the pull request.

[hayeah]

@mkonicek fixed nit and rebased. thanks!

[ghost]

It's been a while since the last commit was reviewed and the labels show this pull request needs review. Based on the blame information for the files in this pull request we identified @korDen as a potential reviewer. Could you take a look please or cc someone with more context?

[thoblr]

Note that this PR is not only related to orientation but fixes a general difference how onLayout events are handled between iOS and Android. See my comment in the in issue #7202 for more info.

[itbrandonsilva]

Any updates on this?

[rigdern]

@mkonicek any update on this PR? My team is using v0.32.0-rc.0 and we're seeing redundant onLayout events on Android as well. We're now being careful when using that event because it can fire so often as to cause performance issues.

Android's onLayout event should only fire when the view's dimensions actually change. This is the way it behaves on iOS.

Adam Comella
Microsoft Corp.

[lacker]

Hey, sorry for letting this sit for a while. The code looks pretty straightforward to me, I think @mkonicek just wanted to double-check if this is the correct behavior. Could you fix that merge conflict?

[mkonicek]

Sorry, I got confused by #7250 mentioning device orientation. I should have read the description more carefully.

Let's ship it, I'll fix the merge conflict if there's one.

@facebook-github-bot shipit

[facebook-github-bot]

@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[mkonicek]

I feel bad I haven't replied on this since May! :( We have an oncall rotation for handling pull requests now, @lacker is oncall and pinged me. Thank you so much Kevin! Hopefully this new oncall rotation will improve things :)

[mkonicek]

@hayeah Looks like this introduced another bug and was partially reverted, see issue #11096, revert: #11222

[hayeah]

looks like it's fixed in #11222

sorry about introducing the bug 😢