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

Android native UI components are not re-layout on dynamically added views #17968

Open
charpeni opened this issue Feb 13, 2018 · 46 comments
Open
Labels
Bug Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Never gets stale Prevent those issues and PRs from getting stale Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Backlog An issue that should be solved at some point, but it's not in the immediate roadmap.

Comments

@charpeni
Copy link
Contributor

charpeni commented Feb 13, 2018

Is this a bug report?

Yes.

Have you read the Contributing Guidelines?

Yes.

Environment

Environment:
OS: macOS High Sierra 10.13.2
Node: 8.9.4
Yarn: 1.3.2
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4443003

Packages: (wanted => installed)
react: 16.3.1 => 16.3.1
react-native: 0.55.4 => 0.55.4

Description

The issue can be noticed if you bridge to React Native the following views:

A view with elements that have a visibility to gone on the initial render won't be displayed after you've set is visibility to visible. view.isShown() will return true, but it will not be there or it will be there but not really re-layout.

A view with elements that are dynamically added, simply by view.addView() or let's say you want to load an image with Fresco it will only work if it was loaded on the initial render.

I've noticed that native components are re-layout on hot reloading, but this.forceUpdate() or changing props won't trigger a re-layout. As an ugly workaround, if we interact with the native component height or width it will trigger a re-layout, so every time you want to toggle a visibility from gone to visible or dynamically adds views you can alter his size.

I've also implemented needsCustomLayoutForChildren without notable change.

Expected Behavior (Native)

Here's the native implementation directly inflated inside an Activity.

Actual Behavior (React Native)

Here's the exact same layout as above, but bridged to react native and inflated inside SimpleViewManager.

Reproducible Demo

https://github.com/charpeni/react-native-android-visibility-issue

Related to #5531 (Already flagged in RN 0.18).

@ardmn

This comment has been minimized.

@charpeni

This comment has been minimized.

@ardmn

This comment has been minimized.

@charpeni

This comment has been minimized.

@ardmn

This comment has been minimized.

@irgendeinich
Copy link

We are running into the same problem while developing the react-native wrapper of our Android SDK.

As far as I can tell the issue is that if you attach a ViewGroup to the react-native view hierarchy any change you make inside this ViewGroup causes a layout pass to be triggered. Layouting on Android happens top-down so the ReactRootView gets the layout calls which it will drop since layouting is handled by the UIManagerModule.

ViewGroupManager already has a needsCustomLayoutForChildren method which tells react-native to call the regular Android layout for any react views you add. There also needs to be a way to tell react-native that we have added a ViewGroup which needs all regular layout calls forwarded. All this would need to do is to call layout() with the already calculated values on your views when ever layout is requested.

@efkan
Copy link

efkan commented Feb 27, 2018

@charpeni what is your Android API?
Is there any different between API 24 and API 25?

I also have an issue that is related with Fresco module.

@charpeni
Copy link
Contributor Author

charpeni commented Mar 1, 2018

@efkan I've only used the default value of a RN project. Also, this bug doesn't only impact Fresco.

@irgendeinich What's your workaround in PSPDFKit?

@irgendeinich
Copy link

@charpeni
What I ended up doing is just calling the layout methods inside a FrameCallback.
I've got something like this in my custom View returned by the ViewManager.

Choreographer.getInstance().postFrameCallback(new Choreographer.FrameCallback() {
    @Override
    public void doFrame(long frameTimeNanos) {
        for (int i = 0; i < getChildCount(); i++) {
            View child = getChildAt(i);
            child.measure(MeasureSpec.makeMeasureSpec(getMeasuredWidth(), MeasureSpec.EXACTLY),
                    MeasureSpec.makeMeasureSpec(getMeasuredHeight(), MeasureSpec.EXACTLY));
            child.layout(0, 0, child.getMeasuredWidth(), child.getMeasuredHeight());
        }
        getViewTreeObserver().dispatchOnGlobalLayout();
    }
    Choreographer.getInstance().postFrameCallback(this);
});

@charpeni charpeni added the Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. label Mar 13, 2018
@react-native-bot react-native-bot added Android Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@react-native-bot react-native-bot added the Platform: macOS Building on macOS. label Mar 20, 2018
@hramos hramos removed the Platform: macOS Building on macOS. label Mar 29, 2018
@leonardossantos
Copy link

Any news about this issue?

@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest release, v0.55?

Thank you for your contributions.

@Tennen
Copy link

Tennen commented May 22, 2018

with v0.55, this bug still appears.

@thainx
Copy link

thainx commented Jun 15, 2018

Is anyone currently working on this issue?

@Dimon70007
Copy link

Dimon70007 commented Jun 20, 2018

it's not a bug it's a feature of react-native, but need more info about update customView in customModule with react-native flow. As workaround you can override requestLayout see here

@Tennen
Copy link

Tennen commented Jun 21, 2018

@Dimon70007 how this could be a feature?

@Dimon70007
Copy link

Dimon70007 commented Jun 22, 2018

react-native does for you all dirty job to reach the best perfomance (but there is almost no documentation in react-native for situation when you want create native lib youself - assumed that you already have a good knoledges about android api) However needsCustomLayoutForChildren method should do this work
See workaround above ^
p.s. sorry for my bad english.

@stale

This comment has been minimized.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Sep 20, 2018
@avonipenti
Copy link

Is this still a bug on RN 0.63? We are using 0.61 and plan to upgrade to 63 soon
I plan to use the workaround but wondering if this is fixed on 0.63?

@speed1speed1
Copy link

No, I used RN 0.63.2 for my custom camera.
I still need to do this for now.

@vKeyGeek
Copy link

vKeyGeek commented Sep 23, 2020

I found a workaround. Whenever you want to refresh layout, you need to call below method with layout parent view argument.

private void refreshViewChildrenLayout(View view){
      view.measure(
                View.MeasureSpec.makeMeasureSpec(view.getMeasuredWidth(), View.MeasureSpec.EXACTLY),
                View.MeasureSpec.makeMeasureSpec(view.getMeasuredHeight(), View.MeasureSpec.EXACTLY));
      view.layout(view.getLeft(), view.getTop(), view.getRight(), view.getBottom());
}

@srfaytkn
Copy link

@vKeyGeek thanks, worked for me

@rossmartin
Copy link

I had this issue on android when creating a wrapper for the mapbox navigation SDK. Here is how I solved it - homeeondemand/react-native-mapbox-navigation@4fceba3

override fun requestLayout() {
    super.requestLayout()

    // This view relies on a measure + layout pass happening after it calls requestLayout().
    // https://github.com/facebook/react-native/issues/4990#issuecomment-180415510
    // https://stackoverflow.com/questions/39836356/react-native-resize-custom-ui-component
    post(measureAndLayout)
}

private val measureAndLayout = Runnable {
    measure(MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY),
            MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY))
    layout(left, top, right, bottom)
}

@Balasnest
Copy link

Balasnest commented Dec 30, 2020

I am creating RN wrapper for cameraX. I am getting ~5 fps rather than the 15 fps using above solution. Is anybody facing frame drops using with workaround solution?

GPU Rendering for RN implementation: ( ~5 fps)
Screenshot 2020-12-30 at 10 01 02 PM

This one clearly showed a huge portion of the rendering was colored in light green, which corresponds to Measure/Layout according to the documentation. @rossmartin solution has reduced more on the rendering stage for now. But there is no improvement on the fps.

GPU Rending for native implementation: (15 fps)
Screenshot 2020-12-30 at 10 12 33 PM

@wuknife
Copy link

wuknife commented Jun 17, 2022

I found a workaround. Whenever you want to refresh layout, you need to call below method with layout parent view argument.

private void refreshViewChildrenLayout(View view){
      view.measure(
                View.MeasureSpec.makeMeasureSpec(view.getMeasuredWidth(), View.MeasureSpec.EXACTLY),
                View.MeasureSpec.makeMeasureSpec(view.getMeasuredHeight(), View.MeasureSpec.EXACTLY));
      view.layout(view.getLeft(), view.getTop(), view.getRight(), view.getBottom());
}

it's worked for me too, thanks

@wood1986
Copy link
Contributor

any update about this bug?

@ilber
Copy link

ilber commented Aug 18, 2022

Unbelievable that this bug is still open.

@github-actions
Copy link

github-actions bot commented Mar 1, 2023

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Mar 1, 2023
@thevoiceless
Copy link

Still a bug

@github-actions github-actions bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Mar 2, 2023
@xiamu14
Copy link

xiamu14 commented Apr 13, 2023

Still a bug

@ankitkaswan24
Copy link

Still a but, confirming again...

@cortinico cortinico added the Never gets stale Prevent those issues and PRs from getting stale label Apr 17, 2023
@iujames
Copy link

iujames commented Aug 9, 2023

Had similar issues trying to auto-size some content from a native Android View being hosted inside of a React Native view. Wanted to share the workaround approach that worked for me - involves similar usage of a measureAndLayout helper from the requestLayout() override, but then utilizing a call to UIManager to updateNodeSize(id, width, height) from the onMeasure override.

In case it helps anyone else...

    override fun requestLayout() {
        super.requestLayout()
        post(measureAndLayout)
    }

    private val measureAndLayout = Runnable {
        measure(
            MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY),
            MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY)
        )
        layout(left, top, right, bottom)
    }

    override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) {
        var maxWidth = 0
        var maxHeight = 0
        for (i in 0 until childCount) {
            val child = getChildAt(i)
            if (child.visibility != GONE) {
                child.measure(widthMeasureSpec, MeasureSpec.UNSPECIFIED)
                maxWidth = maxWidth.coerceAtLeast(child.measuredWidth)
                maxHeight = maxHeight.coerceAtLeast(child.measuredHeight)
            }
        }
        val finalWidth = maxWidth.coerceAtLeast(suggestedMinimumWidth)
        val finalHeight = maxHeight.coerceAtLeast(suggestedMinimumHeight)
        setMeasuredDimension(finalWidth, finalHeight)
        (context as ThemedReactContext).runOnNativeModulesQueueThread {
            (context as ThemedReactContext).getNativeModule(UIManagerModule::class.java)
                ?.updateNodeSize(id, finalWidth, finalHeight)
        }
    }

@mikemilla
Copy link

I was still experiencing this issue in 0.73+...

Solved it by wrapping my View class with a workaround:

internal class MyReactNativeView @JvmOverloads constructor(context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = 0) : MyView(context, attrs, defStyleAttr) {

  override fun requestLayout() {
    super.requestLayout()
    post(measureAndLayout)
  }

  private val measureAndLayout = Runnable {
    measure(
      MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY),
      MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY)
    )
    layout(left, top, right, bottom)
  }

}

classic rn madness

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Never gets stale Prevent those issues and PRs from getting stale Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Backlog An issue that should be solved at some point, but it's not in the immediate roadmap.
Projects
None yet
Development

No branches or pull requests