Fire 'onLayout' when elements are resized #848

giuseppeg · 2018-03-02T22:33:18Z

Fixes #60

Uses ResizeObserver and fallbacks to window.onresize (with initial firing). This should be a backwards compatible change.

I can add some integration tests if you want and think that the PR is good.

giuseppeg · 2018-03-02T22:52:01Z

packages/react-native-web/src/modules/applyLayout/index.js

    }
  );

  Component.prototype.componentDidUpdate = safeOverride(
    componentDidUpdate,
-    function componentDidUpdate() {
-      this._handleLayout();


Actually removing this might break backwards compatibility. Happy to fix this if you think it is necessary.

necolas · 2018-03-02T23:47:21Z

Thanks for taking this on! One question I had when I looked at resize observer is whether the contentRect/measure dimensions are equivalent when the node has padding or border. Do you know?

giuseppeg · 2018-03-03T09:04:07Z

The contentRect for each entry includes x, y, width, height, top, right, bottom, left with top, right, bottom, left being the distances from the element rect. Specifically

The top attribute must return min(y coordinate, y coordinate + height dimension)
The right attribute must return max(x coordinate, x coordinate + width dimension)
The bottom attribute must return max(y coordinate, y coordinate + height dimension)
The left attribute must return min(x coordinate, x coordinate + width dimension)

So for example if we have padding: 10 the rect might look like this:

{
  x: 10,
  y: 10,
  top: 10,
  left: 10,
  right: 310,
  bottom: 210,
  width: 300,
  height: 200
}

I am not familiar with the onLayout api and I don't know why it doesn't include this information (probably because it could be derived). If the expected result is to get the same measurements with or without padding/borders we could just do the math.

Unrelated: the calls to _handleLayout are debounced but I am not sure I like that. Probably onLayout should be debounced in user land, calling _handleLayout as it is now (when using ResizeObserver) shouldn't be as expensive as it was before.

necolas · 2018-03-03T14:27:31Z

if it's returning the layout of the contentRect and not the bounding box, the resize observer path might be returning different results. Agree that debouncing could be removed in this patch

giuseppeg · 2018-03-03T19:49:23Z

just to make sure I understood

if it's returning the layout of the contentRect and not the bounding box, the resize observer path might be returning different results

Are you saying that we need the bounding box (haven't had the chance to check the implementation of measure in RNW to confirm)? In that case we could use entry.target.getBoundingClientRect() instead of entry.contentRect, right?

necolas · 2018-03-03T20:10:20Z

Yeah I'd make a bunch of different boxes with/without padding, border, etc., and compare the results you get in snack.expo.io with the web implementations to make sure it is the same in every case (inc when offset in a scroll view)

giuseppeg · 2018-03-04T11:18:51Z

Indeed using the contentRect or even entry.target.getBoundingClientRect() is not correct.
I noticed that Views create new "layout contexts" therefore when the observed component is inside of a View its x and y coordinates are reset (relative to the new View).

Since the measure method you implemented takes this into account already, I decided to use ResizeObserver only to trigger instance._handleLayout calls. For this reason I am debouncing this function now because it triggers a new measurement which can be expensive (correct me if I am wrong here or think that we can or should not debounce).

necolas · 2018-03-05T19:54:53Z

packages/react-native-web/src/modules/applyLayout/index.js

 const safeOverride = (original, next) => {
  if (original) {
    return function prototypeOverride() {
-      original.call(this);
-      next.call(this);
+      /* eslint-disable prefer-rest-params */


What's the reason for this?

personally I think that it is unnecessary to use the rest operator in this case, this version should perform better than its transpiled equivalent.

necolas · 2018-03-05T19:57:55Z

website/storybook/1-components/View/ViewScreen.js

          <AppText>
            Invoked on mount and layout changes with{' '}
            <Code>{'{ nativeEvent: { layout: { x, y, width, height } } }'}</Code>, where{' '}
            <Code>x</Code> and <Code>y</Code> are the offsets from the parent node.
+          </AppText>,
+          <AppText>
+            NOTE: Behind the hood React Native for Web uses <Code>ResizeObserver</Code> and doesn't


Please could you include ResizeObserver and a link to the polyfill in the list of recommended polyfills in the "Getting started" docs

giuseppeg · 2018-03-06T18:49:41Z

website/guides/getting-started.md

+
+### ResizeObserver
+
+To observe layout changes and fire [`onLayout`](https://facebook.github.io/react-native/docs/view.html#onlayout) when elements are resized, React Native for Web uses the [`ResizeObserver` API](https://wicg.github.io/ResizeObserver/). Browsers that don't support this API require a [polyfill](https://github.com/que-etc/resize-observer-polyfill) for this feature.


I am not a native speaker so there might be errors. Feel free to edit this in line in case you spot any :)

necolas · 2018-04-03T17:45:09Z

packages/react-native-web/src/modules/applyLayout/index.js

+const observe = instance => {
+  if (resizeObserver) {
+    const node = findNodeHandle(instance);
+    node._handleLayout = debounce(instance._handleLayout.bind(instance));


I'm applying this locally and noticed this. It would be good if we could avoid binding a new function to the DOM node and instead use a pointer (like _onLayoutId) to the instance's method. Did you try that? Also wondering why this function is debounced.

Good point, node._onLayoutId and look up the instance in the registry should work.

RE: debouncing

As I said above

Since the measure method you implemented takes this into account already, I decided to use ResizeObserver only to trigger instance._handleLayout calls. For this reason I am debouncing this function now because it triggers a new measurement which can be expensive (correct me if I am wrong here or think that we can or should not debounce).

Measurements will be triggered, whether the user callback is debunced/throttled or not. If we decide to get rid of the debouncing I think that, for the sake of consistency, we should also remove the debouncing here https://github.com/necolas/react-native-web/pull/848/files#diff-fa9d0b1cb86f4e38c30f6685ec719be4R44

That's not debouncing measurements but the frequency with which onresize is called

Right but as a consequence of that measurements (which happen in _handleLayout) will be delayed to when resize ends.

Also wondering why this function is debounced

That was to keep the behavior consistent otherwise, on resize, ResizeObserver would fire continuously and we'd invoke _handleLayout (measure) without delays.

Let me know if I am missing something.

Hmm I see. I wonder if it's helpful to debounce at all. I suspect most viewport resizing is discrete - orientation change, etc - rather than the kind of continuous resizing developers might do. And maybe debouncing is better left to app developers who know when they want to use it? There's also the fact that measurement uses rAF, which needs to change too.

I'm working on a branch for FlatList that I'll push up so you can see how this patch interacts with it. I do want to get this in, it's just surfacing a few existing issues related to layout measurement in the lib :)

And maybe debouncing is better left to app developers who know when they want to use it?

Yes, that would be ideal. With the current implementation though they could only debounce onLayout (the fn prop), measurements would happen regardless. https://github.com/giuseppeg/react-native-web/blob/9db10f12c4dd23c46adb3f33f86932fd57f1f33a/packages/react-native-web/src/modules/applyLayout/index.js#L124-L132

That's true. But probably ok. What I'll do is create a feature branch for this block of work, apply a version of this patch, and then we can work out these other details from there. Thanks for taking the time to understand (and help me understand) the details and for being patient!

No problem! Sounds good, I pushed the change you suggested in #848 (comment) I didn't bother rebasing but I can do that if you want.

…web into observe-onLayout

necolas · 2018-04-08T00:03:52Z

Thanks, this is now in master

necolas · 2018-04-10T15:44:29Z

I asked Twitter to try this out in Twitter Lite and it produced the following error in Chrome for Android

Error: ResizeObserver loop limit exceeded

giuseppeg · 2018-04-10T15:48:13Z

Sounds like the issue I had when I tried nested Views in Expo. Unfortunately I don't have the gist anymore. Can you ask them to try the same or similar hierarchy on Expo/React Native? I am afraid that nested views can trigger infinite onLayout calls or in our case ResizeObserver loop limit exceeded

giuseppeg · 2018-04-10T15:50:11Z

Please keep in mind that I don't have experience with RN so I might as well have done something wrong.

necolas · 2018-04-10T15:50:37Z

WICG/resize-observer#7

home-assistant/frontend#865

giuseppeg · 2018-04-10T16:07:00Z

Would a pause and resume approach work/fix the issue? We could call resizeObserver.disconnect() or our unobserve inside of the callback https://github.com/necolas/react-native-web/pull/848/files#diff-fa9d0b1cb86f4e38c30f6685ec719be4R23 execute the forEach loop and then observe again. Not sure how expensive that can be though.

necolas · 2018-04-10T21:12:43Z

Maybe. We kind of need an example case that triggers the error to iterate against. I only know it's an issue because @paularmstrong saw the error triggered from a build of Twitter Lite using the version of RNW that's in master.

giuseppeg · 2018-04-11T09:10:22Z

I made a testcase https://snack.expo.io/@giu/onlayout-test clicking on "trigger resize" will trigger an infinite loop of updates in cascade (in RN too). The only difference with Web (ResizeObserver) is that it is never interrupted nor throws an error.

giuseppeg added 2 commits March 2, 2018 23:31

Fire 'onLayout' when elements are resized

acf6e66

s/handleResize/_handleLayout

74a405b

giuseppeg commented Mar 2, 2018

View reviewed changes

Invoke override with arguments

9684c6d

Use RNW measure

656a96a

necolas reviewed Mar 5, 2018

View reviewed changes

giuseppeg added 5 commits March 6, 2018 19:47

Fire 'onLayout' when elements are resized

c1810f9

s/handleResize/_handleLayout

cb1437f

Invoke override with arguments

db72e9b

Use RNW measure

4677249

Add note to the Getting started guide

9db10f1

giuseppeg force-pushed the observe-onLayout branch from 656a96a to 9db10f1 Compare March 6, 2018 18:47

giuseppeg commented Mar 6, 2018

View reviewed changes

necolas reviewed Apr 3, 2018

View reviewed changes

giuseppeg added 2 commits April 4, 2018 18:17

Merge branch 'observe-onLayout' of github.com:giuseppeg/react-native-…

fb8d619

…web into observe-onLayout

Hold ref to instance id rather than _handleLayout

5fd6a3c

necolas added this to the 0.6.0 milestone Apr 7, 2018

necolas closed this in 5a04d07 Apr 8, 2018

mattdanielbrown mentioned this pull request May 13, 2022

[Snyk] Fix for 1 vulnerabilities mattdanielbrown/react-native-web#63

Open

mattdanielbrown mentioned this pull request Mar 1, 2023

[Snyk] Fix for 8 vulnerabilities mattdanielbrown/react-native-web#93

Open

mattdanielbrown mentioned this pull request Nov 28, 2023

[Snyk] Fix for 8 vulnerabilities mattdanielbrown/react-native-web#106

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fire 'onLayout' when elements are resized #848

Fire 'onLayout' when elements are resized #848

giuseppeg commented Mar 2, 2018 •

edited

Loading

giuseppeg Mar 2, 2018

giuseppeg Mar 4, 2018

necolas commented Mar 2, 2018

giuseppeg commented Mar 3, 2018 •

edited

Loading

necolas commented Mar 3, 2018

giuseppeg commented Mar 3, 2018

necolas commented Mar 3, 2018

giuseppeg commented Mar 4, 2018

necolas Mar 5, 2018

giuseppeg Mar 5, 2018

necolas Mar 5, 2018

giuseppeg Mar 5, 2018

giuseppeg Mar 6, 2018

necolas Apr 3, 2018

giuseppeg Apr 3, 2018

necolas Apr 3, 2018

giuseppeg Apr 4, 2018

necolas Apr 4, 2018

giuseppeg Apr 4, 2018

necolas Apr 5, 2018

giuseppeg Apr 5, 2018

necolas commented Apr 8, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 11, 2018 •

edited

Loading


		### ResizeObserver

		To observe layout changes and fire [`onLayout`](https://facebook.github.io/react-native/docs/view.html#onlayout) when elements are resized, React Native for Web uses the [`ResizeObserver` API](https://wicg.github.io/ResizeObserver/). Browsers that don't support this API require a [polyfill](https://github.com/que-etc/resize-observer-polyfill) for this feature.

Fire 'onLayout' when elements are resized #848

Fire 'onLayout' when elements are resized #848

Conversation

giuseppeg commented Mar 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

necolas commented Mar 2, 2018

giuseppeg commented Mar 3, 2018 • edited Loading

necolas commented Mar 3, 2018

giuseppeg commented Mar 3, 2018

necolas commented Mar 3, 2018

giuseppeg commented Mar 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

necolas commented Apr 8, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 10, 2018

necolas commented Apr 10, 2018

giuseppeg commented Apr 11, 2018 • edited Loading

giuseppeg commented Mar 2, 2018 •

edited

Loading

giuseppeg commented Mar 3, 2018 •

edited

Loading

giuseppeg commented Apr 11, 2018 •

edited

Loading