Fix updating button options with mergeOptions (#6219)

Updating button options with mergeOptions stopped working in 6.5.0. The regression was introduced by #6090. When updating buttons, RNN didn't take button options into account when checking if two buttons are equal, instead it checked only by button id. closes #6205 Co-authored-by: Yogev Ben David <[email protected]>
wix · May 17, 2020 · b2df65a · b2df65a
1 parent 86b344c
commit b2df65a
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 8 deletions.
diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/presentation/StackPresenter.java b/lib/android/app/src/main/java/com/reactnativenavigation/presentation/StackPresenter.java
@@ -306,10 +306,10 @@ private List<TitleBarButtonController> getOrCreateButtonControllersByInstanceId(
         return new ArrayList<>(result.values());
     }
 
-    private List<TitleBarButtonController> getOrCreateButtonControllersById(@Nullable Map<String, TitleBarButtonController> currentButtons,@NonNull List<Button> buttons) {
+    private List<TitleBarButtonController> getOrCreateButtonControllers(@Nullable Map<String, TitleBarButtonController> currentButtons, @NonNull List<Button> buttons) {
         ArrayList result = new ArrayList<TitleBarButtonController>();
         for (Button b : buttons) {
-            result.add(take(first(perform(currentButtons, null, Map::values), button -> button.getId().equals(b.id)), createButtonController(b)));
+            result.add(take(first(perform(currentButtons, null, Map::values), button -> button.getButton().equals(b)), createButtonController(b)));
         }
         return result;
     }
@@ -369,7 +369,7 @@ private void mergeButtons(TopBarOptions options, TopBarButtons buttons, View chi
     private void mergeRightButtons(TopBarOptions options, TopBarButtons buttons, View child) {
         if (buttons.right == null) return;
         List<Button> rightButtons = mergeButtonsWithColor(buttons.right, options.rightButtonColor, options.rightButtonDisabledColor);
-        List<TitleBarButtonController> toMerge = getOrCreateButtonControllersById(componentRightButtons.get(child), rightButtons);
+        List<TitleBarButtonController> toMerge = getOrCreateButtonControllers(componentRightButtons.get(child), rightButtons);
         List<TitleBarButtonController> toRemove = difference(currentRightButtons, toMerge, TitleBarButtonController::equals);
         forEach(toRemove, TitleBarButtonController::destroy);
 
@@ -383,7 +383,7 @@ private void mergeRightButtons(TopBarOptions options, TopBarButtons buttons, Vie
     private void mergeLeftButton(TopBarOptions options, TopBarButtons buttons, View child) {
         if (buttons.left == null) return;
         List<Button> leftButtons = mergeButtonsWithColor(buttons.left, options.leftButtonColor, options.leftButtonDisabledColor);
-        List<TitleBarButtonController> toMerge = getOrCreateButtonControllersById(componentLeftButtons.get(child), leftButtons);
+        List<TitleBarButtonController> toMerge = getOrCreateButtonControllers(componentLeftButtons.get(child), leftButtons);
         componentLeftButtons.put(child, keyBy(toMerge, TitleBarButtonController::getButtonInstanceId));
         topBarController.setLeftButtons(toMerge);
     }

diff --git a/...app/src/main/java/com/reactnativenavigation/viewcontrollers/TitleBarButtonController.java b/...app/src/main/java/com/reactnativenavigation/viewcontrollers/TitleBarButtonController.java
@@ -29,7 +29,6 @@
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
-import androidx.annotation.RestrictTo;
 import androidx.appcompat.widget.ActionMenuView;
 import androidx.appcompat.widget.Toolbar;
 import androidx.core.view.MenuItemCompat;
@@ -48,7 +47,6 @@ public interface OnClickListener {
     private TitleBarButtonController.OnClickListener onPressListener;
     private Drawable icon;
 
-    @RestrictTo(RestrictTo.Scope.TESTS)
     public Button getButton() {
         return button;
     }

diff --git a/...droid/app/src/test/java/com/reactnativenavigation/viewcontrollers/StackPresenterTest.java b/...droid/app/src/test/java/com/reactnativenavigation/viewcontrollers/StackPresenterTest.java
@@ -2,6 +2,7 @@
 
 import android.app.Activity;
 import android.content.Context;
+import android.graphics.Color;
 import android.graphics.Typeface;
 import android.view.Gravity;
 import android.view.View;
@@ -12,9 +13,9 @@
 import com.reactnativenavigation.mocks.ImageLoaderMock;
 import com.reactnativenavigation.mocks.Mocks;
 import com.reactnativenavigation.mocks.SimpleViewController;
+import com.reactnativenavigation.mocks.TitleBarButtonCreatorMock;
 import com.reactnativenavigation.mocks.TitleBarReactViewCreatorMock;
 import com.reactnativenavigation.mocks.TopBarBackgroundViewCreatorMock;
-import com.reactnativenavigation.mocks.TitleBarButtonCreatorMock;
 import com.reactnativenavigation.parse.Alignment;
 import com.reactnativenavigation.parse.Component;
 import com.reactnativenavigation.parse.Options;
@@ -244,6 +245,7 @@ public void mergeRightButtons_mergingButtonsOnlyDestroysRightButtons() {
     @Test
     public void mergeRightButtons_buttonsAreCreatedOnlyIfNeeded() {
         Options toApply = new Options();
+        textBtn1.color = new Colour(Color.GREEN);
         toApply.topBar.buttons.right = new ArrayList<>(asList(textBtn1, componentBtn1));
         uut.applyChildOptions(toApply, parent, child);
 
@@ -254,6 +256,7 @@ public void mergeRightButtons_buttonsAreCreatedOnlyIfNeeded() {
 
         Options toMerge = new Options();
         toMerge.topBar.buttons.right = new ArrayList(requireNonNull(map(toApply.topBar.buttons.right, Button::copy)));
+        toMerge.topBar.buttons.right.get(0).color = new Colour(Color.RED);
         toMerge.topBar.buttons.right.add(1, componentBtn2);
         uut.mergeChildOptions(toMerge, Options.EMPTY, parent, child);
 
@@ -262,7 +265,7 @@ public void mergeRightButtons_buttonsAreCreatedOnlyIfNeeded() {
         verify(topBarController).mergeRightButtons(captor2.capture(), any());
         List<TitleBarButtonController> mergedButtons = captor2.getValue();
         assertThat(mergedButtons).hasSize(3);
-        assertThat(appliedButtons.get(0)).isEqualTo(mergedButtons.get(0));
+        assertThat(appliedButtons.get(0)).isNotEqualTo(mergedButtons.get(0));
         assertThat(appliedButtons.get(1)).isEqualTo(mergedButtons.get(2));
     }