facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
117.14k stars 24.08k forks source link

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

Open charpeni opened 6 years ago

charpeni commented 6 years ago

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 commented 6 years ago

Same issues . I think the maintainers are have only iOS devices and don't want support android. I think they do work on react-native only because they likes write code on js for iOS.

charpeni commented 6 years ago

@ardmn This issue is unfortunate, but please stay polite and respectful.

ardmn commented 6 years ago

@charpeni Are you think that I am not polite and respectful ? Where ? I'm not insulting anybody. I'm just telling you how I see it. I am only say about my thinks.

charpeni commented 6 years ago

You're not insulting anyone directly but telling how you see it is unnecessary and totally unrelated to the issue. This type of comments won't lead to a resolution, many contributors are working hard to maintain and support Android and they shouldn't be uncomfortable with your perception.

ardmn commented 6 years ago

@charpeni

This type of comments won't lead to a resolution, many contributors are working hard to maintain and support Android

You are right. But why it's work on iOS and nit work on Android ?

This type of comments won't lead to a resolution

I think that more comment can draw more people. More people => more chance to solve problem ;)

irgendeinich commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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);
});
leonardossantos commented 6 years ago

Any news about this issue?

react-native-bot commented 6 years ago

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 commented 6 years ago

with v0.55, this bug still appears.

thainx commented 6 years ago

Is anyone currently working on this issue?

Dimon70007 commented 6 years ago

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 commented 6 years ago

@Dimon70007 how this could be a feature?

Dimon70007 commented 6 years ago

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[bot] commented 5 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

stale[bot] commented 5 years ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

nachodeh commented 5 years ago

Is anyone working on this issue?

musicode commented 5 years ago

with v0.57.7, this bug still appears.

musicode commented 5 years ago
public class RNTViewManager extends ViewGroupManager<RNTView> {

    ...

    @Override
    public boolean needsCustomLayoutForChildren() {
        return true;
    }

}

needsCustomLayoutForChildren return true has no effects. the view added dynamically is still not visible.

how to fix it?

jedi4ever commented 5 years ago

A couple of findings:

Adding the following to the view that gets returned works:

void setupLayoutHack() {

        Choreographer.getInstance().postFrameCallback(new Choreographer.FrameCallback() {
            @Override
            public void doFrame(long frameTimeNanos) {
                manuallyLayoutChildren();
                getViewTreeObserver().dispatchOnGlobalLayout();
                    Choreographer.getInstance().postFrameCallback(this);
            }
        });
    }

    void manuallyLayoutChildren() {
        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());
        }
    }

@irgendeinich thanks for the solution! Wonder if you can explain what this callback actual does, and why/when this does get triggered. And if this is expensive in layout effort when things change on your screen alot (like videoplayer).

Source: https://github.com/PSPDFKit/react-native/blob/8ecba78c024bdffc0cb18c83b7cc68ce61c135d9/android/src/main/java/com/pspdfkit/views/PdfView.java#L144

For the record: if the layout of the immediate children of the rootview change is does do a full relayout (for example on rotation of the device) by default (without this hack)

alaeddineG commented 4 years ago

I confirm this is happening in 0.59.9 as well. @jedi4ever works for me. Thanks.

ikimiler commented 4 years ago

I confirm this is happening in 0.59.9 as well. @jedi4ever works for me. Thanks. me too,with rn v0.59.9,the above method is not solved,help me。

stale[bot] commented 4 years ago

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

gabrielpopa commented 4 years ago

For anyone interested I found a better hack to to fix the re-layout not being called on your custom View.

getViewTreeObserver().addOnGlobalLayoutListener(new ViewTreeObserver.OnGlobalLayoutListener() {
   @Override
   public void onGlobalLayout() {
       requestLayout();
   }
});

Add the above code in your custom View constructor. I have also tried @jedi4ever hack, but while it works I don't prefer it because the hack calls for a redraw of the whole tree endlessly.

dev-event commented 4 years ago

Same issue with view rendering in React Native. Thanks. There are those who in custom view created fragments?

iagormoraes commented 4 years ago

For anyone interested I found a better hack to to fix the re-layout not being called on your custom View.

getViewTreeObserver().addOnGlobalLayoutListener(new ViewTreeObserver.OnGlobalLayoutListener() {
   @Override
   public void onGlobalLayout() {
       requestLayout();
   }
});

Add the above code in your custom View constructor. I have also tried @jedi4ever hack, but while it works I don't prefer it because the hack calls for a redraw of the whole tree endlessly.

Your solution didn't work for me, I've placed it to constructor but even dough the listener was not being called when I add a view to a parent view.

fabOnReact commented 4 years ago

I reply to @jedi4ever post:

Choreographer.getInstance().postFrameCallback(new Choreographer.FrameCallback() {
            @Override
            public void doFrame(long frameTimeNanos) { }
})

Reading quickly the Choreographer docs, this may be connected to his applications requirements. He may be drawing using Hardware Accelleration (OpenGL) and may need to take in considerations those graphical operations. I am not sure about this, but I believe also react native uses OpenGL graphical accelleration. Given my basic understanding of Android Sdk you can compute views on software or hardware layer (hardware layer mean using graphic card).

However, there are a few cases where you might want to use the functions of the choreographer directly in your application. Here are some examples.

If your application does its rendering in a different thread, possibly using GL, or does not use the animation framework or view hierarchy at all and you want to ensure that it is appropriately synchronized with the display, then use Choreographer#postFrameCallback. ... and that's about it.

Each Looper thread has its own choreographer. Other threads can post callbacks to run on the choreographer but they will run on the Looper to which the choreographer belongs.

postFrameCallback

Posts a frame callback to run on the next frame.

The callback runs once then is automatically removed. Parameters

--
callback | Choreographer.FrameCallback: The frame callback to run during the next frame.

doFrame

Called when a new display frame is being rendered.

This method provides the time in nanoseconds when the frame started being rendered. The frame time provides a stable time base for synchronizing animations and drawing. It should be used instead of SystemClock#uptimeMillis() or System#nanoTime() for animations and drawing in the UI. Using the frame time helps to reduce inter-frame jitter because the frame time is fixed at the time the frame was scheduled to start, regardless of when the animations or drawing callback actually runs. All callbacks that run as part of rendering a frame will observe the same frame time so using the frame time also helps to synchronize effects that are performed by different callbacks.

Please note that the framework already takes care to process animations and drawing using the frame time as a stable time base. Most applications should not need to use the frame time information directly.

Inside doFrame he calls manuallyLayoutChildren.

void manuallyLayoutChildren() {
    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());
    }
}

It seems to me something really hard to implement inside react-native. I believe would be easier starting to write better documentation on how to implement native modules and ui components...

janselv commented 4 years ago

I can confirm that the issue is still present in version 0.62.2.

I faced this same situation in another scenario but of the same nature and @irgendeinich trick pointed me in the right direction but instead of repeatedly updating the view hierarchy I installed a OnHierarchyChangeListener to the parent view and whenever the view was added/removed to the parent I triggered the manual re-layout on the parent view group. I ended up with something like (kotlin code):

private fun installHierarchyFitter(view: ViewGroup) {
  if (context is ThemedReactContext) { // only react-native setup
    view.setOnHierarchyChangeListener(object : OnHierarchyChangeListener{
      override fun onChildViewRemoved(parent: View?, child: View?) = Unit
      override fun onChildViewAdded(parent: View?, child: View?) {
        parent?.measure(
          MeasureSpec.makeMeasureSpec(measuredWidth, MeasureSpec.EXACTLY),
          MeasureSpec.makeMeasureSpec(measuredHeight, MeasureSpec.EXACTLY)
        )
        parent?.layout(0, 0, parent.measuredWidth, parent.measuredHeight)
      }
    })
  }
}
hengkx commented 3 years ago

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); });

How to use kotlin?

avonipenti commented 3 years ago

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 commented 3 years ago

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

vKeyGeek commented 3 years ago

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 commented 3 years ago

@vKeyGeek thanks, worked for me

rossmartin commented 3 years ago

I had this issue on android when creating a wrapper for the mapbox navigation SDK. Here is how I solved it - https://github.com/homeeondemand/react-native-mapbox-navigation/commit/4fceba358224cb3bc5ee9a649320723c4dea0ea8

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 commented 3 years ago

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 commented 2 years ago

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 commented 1 year ago

any update about this bug?

ilber commented 1 year ago

Unbelievable that this bug is still open.

github-actions[bot] commented 1 year ago

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.

thevoiceless commented 1 year ago

Still a bug

xiamu14 commented 1 year ago

Still a bug

ankitkaswan24 commented 1 year ago

Still a but, confirming again...

iujames commented 11 months ago

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 commented 2 months ago

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