Skip to content

Commit

Permalink
Fix title component not being replaced via mergeOptions (wix#6066)
Browse files Browse the repository at this point in the history
This commit fixes a bug related to setting a new react component as title via mergeOptions. When a title component was set, and the user tried to replace the existing title with a new title via mergeOptions, the new title was not created and instead the current title was reapplied.
Fixes wix#5377
  • Loading branch information
guyca authored and pawlitos committed Apr 8, 2020
1 parent c86575d commit 40d825a
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,13 @@ private void applyTopBarOptions(Options options, StackController stack, ViewCont

if (topBarOptions.title.component.hasValue()) {
if (titleControllers.containsKey(component)) {
topBar.setTitleComponent(titleControllers.get(component).getView());
topBarController.setTitleComponent(titleControllers.get(component));
} else {
TitleBarReactViewController controller = new TitleBarReactViewController(activity, titleViewCreator);
TitleBarReactViewController controller = new TitleBarReactViewController(activity, titleViewCreator, topBarOptions.title.component);
controller.setWaitForRender(topBarOptions.title.component.waitForRender);
titleControllers.put(component, controller);
controller.setComponent(topBarOptions.title.component);
controller.getView().setLayoutParams(getComponentLayoutParams(topBarOptions.title.component));
topBar.setTitleComponent(controller.getView());
topBarController.setTitleComponent(controller);
}
}

Expand Down Expand Up @@ -415,19 +414,19 @@ private void mergeTopBarOptions(Options options, StackController stack, ViewCont
}

if (topBarOptions.title.height.hasValue()) topBar.setTitleHeight(topBarOptions.title.height.get());
if (topBarOptions.title.text.hasValue()) topBar.setTitle(topBarOptions.title.text.get());
if (topBarOptions.title.topMargin.hasValue()) topBar.setTitleTopMargin(topBarOptions.title.topMargin.get());

if (topBarOptions.title.component.hasValue()) {
if (titleControllers.containsKey(component)) {
topBar.setTitleComponent(titleControllers.get(component).getView());
} else {
TitleBarReactViewController controller = new TitleBarReactViewController(activity, titleViewCreator);
titleControllers.put(component, controller);
controller.setComponent(topBarOptions.title.component);
TitleBarReactViewController controller = findTitleComponent(topBarOptions.title.component);
if (controller == null) {
controller = new TitleBarReactViewController(activity, titleViewCreator, topBarOptions.title.component);
perform(titleControllers.put(component, controller), ViewController::destroy);
controller.getView().setLayoutParams(getComponentLayoutParams(topBarOptions.title.component));
topBar.setTitleComponent(controller.getView());
}
topBarController.setTitleComponent(controller);
} else if (topBarOptions.title.text.hasValue()) {
perform(titleControllers.remove(component), ViewController::destroy);
topBar.setTitle(topBarOptions.title.text.get());
}

if (topBarOptions.title.color.hasValue()) topBar.setTitleTextColor(topBarOptions.title.color.get());
Expand Down Expand Up @@ -478,6 +477,16 @@ private void mergeTopBarOptions(Options options, StackController stack, ViewCont
}
}

private TitleBarReactViewController findTitleComponent(com.reactnativenavigation.parse.Component component) {
for (TitleBarReactViewController controller : titleControllers.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;
}
}
return null;
}

private void mergeTopTabsOptions(TopTabsOptions options) {
if (options.selectedTabColor.hasValue() && options.unselectedTabColor.hasValue()) topBar.applyTopTabsColors(options.selectedTabColor, options.unselectedTabColor);
if (options.fontSize.hasValue()) topBar.applyTopTabsFontSize(options.fontSize);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@
public class TitleBarReactViewController extends ViewController<TitleBarReactView> {

private final TitleBarReactViewCreator reactViewCreator;
private Component component;
private final Component component;

public TitleBarReactViewController(Activity activity, TitleBarReactViewCreator reactViewCreator) {
public Component getComponent() {
return component;
}

public TitleBarReactViewController(Activity activity, TitleBarReactViewCreator reactViewCreator, Component component) {
super(activity, CompatUtils.generateViewId() + "", new YellowBoxDelegate(), new Options(), new ViewControllerOverlay(activity));
this.reactViewCreator = reactViewCreator;
this.component = component;
}

@Override
Expand Down Expand Up @@ -49,8 +54,4 @@ public void sendOnNavigationButtonPressed(String buttonId) {
public String getCurrentComponentName() {
return null;
}

public void setComponent(Component component) {
this.component = component;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.reactnativenavigation.anim.TopBarAnimator;
import com.reactnativenavigation.parse.AnimationOptions;
import com.reactnativenavigation.utils.CompatUtils;
import com.reactnativenavigation.viewcontrollers.TitleBarReactViewController;
import com.reactnativenavigation.views.StackLayout;
import com.reactnativenavigation.views.topbar.TopBar;

Expand Down Expand Up @@ -93,4 +94,8 @@ public void resetViewProperties() {
topBar.setRotationY(0);
topBar.setRotation(0);
}

public void setTitleComponent(TitleBarReactViewController component) {
topBar.setTitleComponent(component.getView());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static com.reactnativenavigation.utils.UiUtils.runOnPreDrawOnce;
import static com.reactnativenavigation.utils.ViewUtils.findChildByClass;
import static com.reactnativenavigation.utils.ViewUtils.findChildrenByClass;
import static com.reactnativenavigation.utils.ViewUtils.removeFromParent;

@SuppressLint("ViewConstructor")
public class TitleBar extends Toolbar {
Expand Down Expand Up @@ -70,6 +71,7 @@ public String getTitle() {
}

public void setComponent(View component) {
if (this.component == component) return;
clearTitle();
clearSubtitle();
this.component = component;
Expand Down Expand Up @@ -175,7 +177,7 @@ private void clearSubtitle() {

private void clearComponent() {
if (component != null) {
removeView(component);
removeFromParent(component);
component = null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ public static Button reactViewButton(String name) {
return button;
}

public static Component titleComponent(String componentId) {
Component component = new Component();
component.componentId = new Text(componentId);
component.name = new Text(componentId);
return component;
}

public static Button iconButton(String id, String icon) {
Button button = new Button();
button.id = "someButton";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ public class StackPresenterTest extends BaseTest {
private Button textBtn2 = TitleBarHelper.textualButton("btn2");
private Button componentBtn1 = TitleBarHelper.reactViewButton("btn1_");
private Button componentBtn2 = TitleBarHelper.reactViewButton("btn2_");
private Component titleComponent1 = TitleBarHelper.titleComponent("component1");
private Component titleComponent2 = TitleBarHelper.titleComponent("component2");
private TopBarController topBarController;
private ChildControllersRegistry childRegistry;
private IconResolver iconResolver;

@Override
public void beforeEach() {
Expand All @@ -98,7 +99,7 @@ public TitleBarReactView create(Activity activity, String componentId, String co
}
};
renderChecker = spy(new RenderChecker());
iconResolver = new IconResolver(activity, ImageLoaderMock.mock());
IconResolver iconResolver = new IconResolver(activity, ImageLoaderMock.mock());
uut = spy(new StackPresenter(activity, titleViewCreator, new TopBarBackgroundViewCreatorMock(), new TopBarButtonCreatorMock(), iconResolver, renderChecker, new Options()));
createTopBarController();

Expand Down Expand Up @@ -309,6 +310,59 @@ public void mergeTopBarOptions() {
uut.mergeChildOptions(options, EMPTY_OPTIONS, parent, child);
}

@Test
public void applyTopBarOptions_setTitleComponent() {
Options applyComponent = new Options();
applyComponent.topBar.title.component.name = new Text("Component1");
applyComponent.topBar.title.component.componentId = new Text("Component1id");
uut.applyChildOptions(applyComponent, parent, child);
verify(topBarController).setTitleComponent(any());
}

@Test
public void mergeTopBarOptions_settingTitleDestroysComponent() {
Options componentOptions = new Options();
componentOptions.topBar.title.component = titleComponent1;
uut.applyChildOptions(componentOptions, parent, child);
ArgumentCaptor<TitleBarReactViewController> applyCaptor = ArgumentCaptor.forClass(TitleBarReactViewController.class);
verify(topBarController).setTitleComponent(applyCaptor.capture());

Options titleOptions = new Options();
titleOptions.topBar.title.text = new Text("Some title");
uut.mergeChildOptions(titleOptions, Options.EMPTY, parent, child);
assertThat(applyCaptor.getValue().isDestroyed()).isTrue();
}

@Test
public void mergeTopBarOptions_doesNotRecreateTitleComponentIfEquals() {
Options options = new Options();
options.topBar.title.component = titleComponent1;
uut.applyChildOptions(options, parent, child);
ArgumentCaptor<TitleBarReactViewController> applyCaptor = ArgumentCaptor.forClass(TitleBarReactViewController.class);
verify(topBarController).setTitleComponent(applyCaptor.capture());

uut.mergeChildOptions(options, Options.EMPTY, parent, child);
verify(topBarController, times(2)).setTitleComponent(applyCaptor.getValue());
}

@Test
public void mergeTopBarOptions_previousTitleComponentIsDestroyed() {
Options options = new Options();
options.topBar.title.component = titleComponent1;
uut.applyChildOptions(options, parent, child);
ArgumentCaptor<TitleBarReactViewController> applyCaptor = ArgumentCaptor.forClass(TitleBarReactViewController.class);
verify(topBarController).setTitleComponent(applyCaptor.capture());

Options toMerge = new Options();
toMerge.topBar.title.component = titleComponent2;
uut.mergeChildOptions(toMerge, Options.EMPTY, parent, child);
ArgumentCaptor<TitleBarReactViewController> mergeCaptor = ArgumentCaptor.forClass(TitleBarReactViewController.class);
verify(topBarController, times(2)).setTitleComponent(mergeCaptor.capture());

assertThat(applyCaptor.getValue()).isNotEqualTo(mergeCaptor.getValue());
assertThat(applyCaptor.getValue().isDestroyed()).isTrue();
}

@Test
public void mergeTopTabsOptions() {
Options options = new Options();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ public class TitleBarReactViewControllerTest extends BaseTest {
public void beforeEach() {
viewCreator = spy(new TitleBarReactViewCreatorMock());
activity = newActivity();
uut = new TitleBarReactViewController(activity, viewCreator);
createComponent();
uut.setComponent(component);
component = createComponent();
uut = new TitleBarReactViewController(activity, viewCreator, component);
}

@Test
Expand All @@ -34,9 +33,10 @@ public void createView() {
verify(viewCreator).create(activity, component.componentId.get(), component.name.get());
}

private void createComponent() {
component = new Component();
private Component createComponent() {
Component component = new Component();
component.componentId = new Text("compId");
component.name = new Text("compName");
return component;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,15 @@ public void setComponent_addsComponentToTitleBar() {
verify(uut).addView(component);
}

@Test
public void setComponent_doesNothingIfComponentIsAlreadyAdded() {
View component = new View(activity);
uut.setComponent(component);

uut.setComponent(component);
verify(uut).addView(component);
}

@Test
public void addView_overflowIsEnabledForButtonsContainer() {
ActionMenuView buttonsContainer = mock(ActionMenuView.class);
Expand All @@ -141,7 +150,7 @@ public void clear() {
assertThat(uut.getTitle()).isNullOrEmpty();
assertThat(uut.getMenu().size()).isZero();
assertThat(uut.getNavigationIcon()).isNull();
verify(uut).removeView(title);
assertThat(title.getParent()).isNull();
}

@Test
Expand Down
1 change: 0 additions & 1 deletion playground/src/commons/Options.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const Colors = require('./Colors');
const { Dimensions } = require('react-native');
const height = Math.round(Dimensions.get('window').height);
const width = Math.round(Dimensions.get('window').width);
console.log('guyca', `height: ${height} width: ${width}`);
const {
useCustomAnimations,
useSlowOpenScreenAnimations,
Expand Down

0 comments on commit 40d825a

Please sign in to comment.