Skip to content

Commit

Permalink
Fix Topbar custom component flicker across screens #3864 (#5110)
Browse files Browse the repository at this point in the history
When a topBar.backgroundComponent is defined, check if a corresponding ReactView has already been created with matching componentName and componentId. If such view is found, reuse it instead of creating another view.

This change will fix a flickering in the TopBar background component when a screen was pushed into a stack with a background component.
  • Loading branch information
FRizzonelli authored and guyca committed May 21, 2019
1 parent 9a903ce commit 99032e0
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 96 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.reactnativenavigation.parse.params;

import android.support.annotation.NonNull;

public class Text extends Param<String> {
public Text(String value) {
super(value);
}

@NonNull
@Override
public String toString() {
return hasValue() ? value : "No Value";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@
import java.util.List;
import java.util.Map;

import static com.reactnativenavigation.utils.CollectionUtils.filter;
import static com.reactnativenavigation.utils.CollectionUtils.forEach;
import static com.reactnativenavigation.utils.CollectionUtils.keyBy;
import static com.reactnativenavigation.utils.CollectionUtils.merge;
import static com.reactnativenavigation.utils.CollectionUtils.*;
import static com.reactnativenavigation.utils.ObjectUtils.perform;

public class StackPresenter {
Expand Down Expand Up @@ -205,8 +202,9 @@ private void applyTopBarOptions(TopBarOptions options, AnimationsOptions animati
topBar.setBackgroundColor(options.background.color.get(Color.WHITE));

if (options.background.component.hasValue()) {
if (backgroundControllers.containsKey(component)) {
topBar.setBackgroundComponent(backgroundControllers.get(component).getView());
View createdComponent = findBackgroundComponent(options.background.component);
if (createdComponent != null) {
topBar.setBackgroundComponent(createdComponent);
} else {
TopBarBackgroundViewController controller = new TopBarBackgroundViewController(activity, topBarBackgroundViewCreator);
controller.setWaitForRender(options.background.waitForRender);
Expand All @@ -215,6 +213,8 @@ private void applyTopBarOptions(TopBarOptions options, AnimationsOptions animati
controller.getView().setLayoutParams(new RelativeLayout.LayoutParams(FrameLayout.LayoutParams.MATCH_PARENT, FrameLayout.LayoutParams.MATCH_PARENT));
topBar.setBackgroundComponent(controller.getView());
}
} else {
topBar.clearBackgroundComponent();
}

if (options.testId.hasValue()) topBar.setTestId(options.testId.get());
Expand All @@ -233,6 +233,17 @@ private void applyTopBarOptions(TopBarOptions options, AnimationsOptions animati
}
}

@Nullable
private View findBackgroundComponent(com.reactnativenavigation.parse.Component component) {
for (TopBarBackgroundViewController controller : backgroundControllers.values()) {
if (ObjectUtils.equalsNotNull(controller.getComponent().name.get(null), component.name.get(null)) &&
ObjectUtils.equalsNotNull(controller.getComponent().componentId.get(null), component.componentId.get(null))) {
return controller.getView();
}
}
return null;
}

private void setInitialTopBarVisibility(TopBarOptions options) {
if (options.visible.isFalse()) {
topBar.hide();
Expand Down Expand Up @@ -268,15 +279,15 @@ private void applyButtons(TopBarOptions options, Component child) {
componentRightButtons.put(child, keyBy(rightButtonControllers, TitleBarButtonController::getButtonInstanceId));
topBar.setRightButtons(rightButtonControllers);
} else {
topBar.setRightButtons(null);
topBar.clearRightButtons();
}

if (leftButtons != null) {
List<TitleBarButtonController> leftButtonControllers = getOrCreateButtonControllers(componentLeftButtons.get(child), leftButtons);
componentLeftButtons.put(child, keyBy(leftButtonControllers, TitleBarButtonController::getButtonInstanceId));
topBar.setLeftButtons(leftButtonControllers);
} else {
topBar.setLeftButtons(null);
topBar.clearLeftButtons();
}

if (options.buttons.back.visible.isTrue() && !options.buttons.hasLeftButtons()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,8 @@ public static <T, S> S perform(@Nullable T obj, S defaultValue, FuncR1<T, S> act
public static boolean notNull(Object o) {
return o != null;
}

public static <T> boolean equalsNotNull(@Nullable T a, @Nullable T b) {
return a != null && a.equals(b);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,6 @@ public void mergeChildOptions(Options options, ViewController childController, C
);
}

@Override
public void destroy() {
topBarController.clear();
super.destroy();
}

@Override
public void clearOptions() {
super.clearOptions();
topBarController.clear();
}

@Override
public void onChildDestroyed(Component child) {
super.onChildDestroyed(child);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,8 @@ public void sendOnNavigationButtonPressed(String buttonId) {
public void setComponent(Component component) {
this.component = component;
}

public Component getComponent() {
return component;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@ protected TopBar createTopBar(Context context, StackLayout stackLayout) {
return new TopBar(context, stackLayout);
}

public void clear() {
if (topBar != null) {
topBar.clear();
}
}

public TopBar getView() {
return topBar;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.reactnativenavigation.views.titlebar.TitleBar;
import com.reactnativenavigation.views.toptabs.TopTabs;

import java.util.Collections;
import java.util.List;

import static android.view.ViewGroup.LayoutParams.MATCH_PARENT;
Expand Down Expand Up @@ -174,6 +175,7 @@ public void setTitleComponent(View component) {
}

public void setBackgroundComponent(View component) {
if (this.component == component || component.getParent() != null) return;
this.component = component;
root.addView(component, 0);
}
Expand Down Expand Up @@ -208,10 +210,18 @@ public void setLeftButtons(List<TitleBarButtonController> leftButtons) {
titleBar.setLeftButtons(leftButtons);
}

public void clearLeftButtons() {
titleBar.setLeftButtons(Collections.emptyList());
}

public void setRightButtons(List<TitleBarButtonController> rightButtons) {
titleBar.setRightButtons(rightButtons);
}

public void clearRightButtons() {
titleBar.setRightButtons(Collections.emptyList());
}

public void setElevation(Double elevation) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && getElevation() != elevation.floatValue()) {
this.elevation = UiUtils.dpToPx(getContext(), elevation.floatValue());
Expand Down Expand Up @@ -271,12 +281,11 @@ public void hideAnimate(AnimationOptions options, Runnable onAnimationEnd) {
animator.hide(options, onAnimationEnd);
}

public void clear() {
public void clearBackgroundComponent() {
if (component != null) {
root.removeView(component);
component = null;
}
titleBar.clear();
}

public void clearTopTabs() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.json.JSONObject;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -354,6 +355,19 @@ public void applyButtons_buttonColorIsMergedToButtons() {
assertThat(leftCaptor.getValue().get(0)).isNotEqualTo(leftButton);
}

@Test
public void applyTopBarOptions_backgroundComponentIsCreatedOnceIfNameAndIdAreEqual() {
Options o = new Options();
o.topBar.background.component.name = new Text("comp");
o.topBar.background.component.componentId = new Text("compId");

uut.applyChildOptions(o, Mockito.mock(com.reactnativenavigation.views.Component.class));
assertThat(uut.getBackgroundComponents().size()).isOne();

uut.applyChildOptions(o, Mockito.mock(com.reactnativenavigation.views.Component.class));
assertThat(uut.getBackgroundComponents().size()).isOne();
}

@Test
public void mergeChildOptions_buttonColorIsResolvedFromAppliedOptions() {
Options appliedOptions = new Options();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -1036,12 +1036,6 @@ public void resolvedOptionsAreAppliedWhenStackIsAttachedToParentAndNotVisible()
verify(presenter).applyChildOptions(any(), eq(component));
}

@Test
public void destroy() {
uut.destroy();
verify(topBarController, times(1)).clear();
}

private void assertContainsOnlyId(String... ids) {
assertThat(uut.size()).isEqualTo(ids.length);
assertThat(uut.getChildControllers()).extracting((Extractor<ViewController, String>) ViewController::getId).containsOnly(ids);
Expand Down

0 comments on commit 99032e0

Please sign in to comment.