Create bottom tabs once (#4478)

Create BottomTabs once when options are applied

The BottomTabs library we use recreates its children each time a style property changes.
This can hinder performance quite a bit as the view creation logic is quite costly.
This commit introduces a simple fix to this issue - we delay view creation until all options are applied.

BottomTabs refactor
* Prevent view recreation in `onSizeChanged`
* Cleanup `createTabs` method

lib/android/app/build.gradle

@@ -87,7 +87,7 @@ dependencies {
     implementation 'com.android.support:appcompat-v7:26.1.0'
     implementation 'com.android.support:support-v4:26.1.0'
 
-    implementation 'com.github.wix-playground:ahbottomnavigation:2.4.6'
+    implementation 'com.github.wix-playground:ahbottomnavigation:2.4.8'
     implementation 'com.github.wix-playground:reflow-animator:1.0.4'
     implementation 'com.github.clans:fab:1.6.4'
 

lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabPresenter.java

@@ -45,7 +45,7 @@ public class BottomTabPresenter {
         this.bottomTabs = bottomTabs;
     }
 
-    public void present() {
+    public void applyOptions() {
         for (int i = 0; i < tabs.size(); i++) {
             BottomTabOptions tab = tabs.get(i).options.copy().withDefaultOptions(defaultOptions).bottomTabOptions;
             bottomTabs.setBadge(i, tab.badge.get(""));

lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabsPresenter.java

@@ -52,7 +52,7 @@ public class BottomTabsPresenter {
         mergeBottomTabsOptions(options.bottomTabsOptions, options.animations);
     }
 
-    public void present(Options options) {
+    public void applyOptions(Options options) {
         Options withDefaultOptions = options.copy().withDefaultOptions(defaultOptions);
         applyBottomTabsOptions(withDefaultOptions.bottomTabsOptions, withDefaultOptions.animations);
     }

lib/android/app/src/main/java/com/reactnativenavigation/utils/ImageLoader.java

@@ -33,6 +33,16 @@ public class ImageLoader {
 
     private static final String FILE_SCHEME = "file";
 
+    @Nullable
+    public Drawable loadIcon(Context context, String uri) {
+        try {
+            return getDrawable(context, uri);
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+        return null;
+    }
+
     public void loadIcon(Context context, String uri, ImagesLoadingListener listener) {
         try {
             listener.onComplete(getDrawable(context, uri));

lib/android/app/src/main/java/com/reactnativenavigation/utils/Time.java

@@ -29,7 +29,7 @@ public class Time {
 
     public static void log(String tag) {
         if (tagsToStartTime.containsKey(tag)) {
-            Log.i(tag, "Elapsed: " + (now() - time(tag)));
+            Log.i(tag, "Elapsed: " + (now() - time(tag)) + "ms");
         } else {
             Log.v(tag, "logging start");
         }

lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/bottomtabs/BottomTabsController.java

@@ -1,7 +1,6 @@
 package com.reactnativenavigation.viewcontrollers.bottomtabs;
 
 import android.app.Activity;
-import android.graphics.drawable.Drawable;
 import android.support.annotation.NonNull;
 import android.support.annotation.RestrictTo;
 import android.view.View;
@@ -18,20 +17,20 @@ import com.reactnativenavigation.presentation.Presenter;
 import com.reactnativenavigation.react.EventEmitter;
 import com.reactnativenavigation.utils.CommandListener;
 import com.reactnativenavigation.utils.ImageLoader;
-import com.reactnativenavigation.utils.ImageLoadingListenerAdapter;
 import com.reactnativenavigation.viewcontrollers.ChildControllersRegistry;
 import com.reactnativenavigation.viewcontrollers.ParentController;
 import com.reactnativenavigation.viewcontrollers.ViewController;
 import com.reactnativenavigation.views.BottomTabs;
 import com.reactnativenavigation.views.Component;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
 import static android.view.ViewGroup.LayoutParams.MATCH_PARENT;
 import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT;
 import static android.widget.RelativeLayout.ALIGN_PARENT_BOTTOM;
+import static com.reactnativenavigation.utils.CollectionUtils.forEach;
+import static com.reactnativenavigation.utils.CollectionUtils.map;
 
 public class BottomTabsController extends ParentController implements AHBottomNavigation.OnTabSelectedListener, TabSelector {
 
@@ -49,6 +48,7 @@ public class BottomTabsController extends ParentController implements AHBottomNa
         this.imageLoader = imageLoader;
         this.presenter = bottomTabsPresenter;
         this.tabPresenter = bottomTabPresenter;
+        forEach(tabs, (tab) -> tab.setParentController(this));
     }
 
     @Override
@@ -62,22 +62,30 @@ public class BottomTabsController extends ParentController implements AHBottomNa
 	@Override
 	protected ViewGroup createView() {
 		RelativeLayout root = new RelativeLayout(getActivity());
-		bottomTabs = new BottomTabs(getActivity());
+		bottomTabs = createBottomTabs();
         presenter.bindView(bottomTabs, this);
         tabPresenter.bindView(bottomTabs);
         bottomTabs.setOnTabSelectedListener(this);
 		RelativeLayout.LayoutParams lp = new RelativeLayout.LayoutParams(MATCH_PARENT, WRAP_CONTENT);
 		lp.addRule(ALIGN_PARENT_BOTTOM);
 		root.addView(bottomTabs, lp);
-		createTabs(root);
-		return root;
+		bottomTabs.addItems(createTabs());
+        attachTabs(root);
+        return root;
 	}
 
+    @NonNull
+    protected BottomTabs createBottomTabs() {
+        return new BottomTabs(getActivity());
+    }
+
     @Override
     public void applyOptions(Options options) {
         super.applyOptions(options);
-        presenter.present(options);
-        tabPresenter.present();
+        bottomTabs.disableItemsCreation();
+        presenter.applyOptions(options);
+        tabPresenter.applyOptions();
+        bottomTabs.enableItemsCreation();
         this.options.bottomTabsOptions.clearOneTimeOptions();
         this.initialOptions.bottomTabsOptions.clearOneTimeOptions();
     }
@@ -135,43 +143,15 @@ public class BottomTabsController extends ParentController implements AHBottomNa
         return false;
 	}
 
-	private void createTabs(RelativeLayout root) {
-		if (tabs.size() > 5) {
-			throw new RuntimeException("Too many tabs!");
-		}
-        List<String> icons = new ArrayList<>();
-        List<BottomTabOptions> bottomTabOptionsList = new ArrayList<>();
-        for (int i = 0; i < tabs.size(); i++) {
-            tabs.get(i).setParentController(this);
-            BottomTabOptions tabOptions = tabs.get(i).resolveCurrentOptions().bottomTabOptions;
-            if (!tabOptions.icon.hasValue()) {
-                throw new RuntimeException("BottomTab must have an icon");
-            }
-            bottomTabOptionsList.add(tabOptions);
-            icons.add(tabOptions.icon.get());
-        }
-
-        imageLoader.loadIcons(getActivity(), icons, new ImageLoadingListenerAdapter() {
-
-            @Override
-            public void onComplete(@NonNull List<Drawable> drawables) {
-                List<AHBottomNavigationItem> tabs = new ArrayList<>();
-                for (int i = 0; i < drawables.size(); i++) {
-                    tabs.add(new AHBottomNavigationItem(bottomTabOptionsList.get(i).text.get(""), drawables.get(i)));
-                }
-                bottomTabs.addItems(tabs);
-                bottomTabs.post(() -> {
-                    for (int i = 0; i < bottomTabOptionsList.size(); i++) {
-                        bottomTabs.setTabTestId(i, bottomTabOptionsList.get(i).testId);
-                    }
-                });
-                attachTabs(root);
-            }
-
-            @Override
-            public void onError(Throwable error) {
-                error.printStackTrace();
-            }
+	private List<AHBottomNavigationItem> createTabs() {
+		if (tabs.size() > 5) throw new RuntimeException("Too many tabs!");
+        return map(tabs, tab -> {
+            BottomTabOptions options = tab.resolveCurrentOptions().bottomTabOptions;
+            return new AHBottomNavigationItem(
+                    options.text.get(""),
+                    imageLoader.loadIcon(getActivity(), options.icon.get()),
+                    options.testId.get("")
+            );
         });
 	}
 

lib/android/app/src/main/java/com/reactnativenavigation/views/BottomTabs.java

@@ -8,26 +8,44 @@ import android.support.annotation.IntRange;
 
 import com.aurelhubert.ahbottomnavigation.AHBottomNavigation;
 import com.aurelhubert.ahbottomnavigation.AHBottomNavigationItem;
-import com.reactnativenavigation.BuildConfig;
-import com.reactnativenavigation.parse.params.Text;
 import com.reactnativenavigation.utils.CompatUtils;
 
-import static com.reactnativenavigation.utils.ObjectUtils.perform;
-
 @SuppressLint("ViewConstructor")
 public class BottomTabs extends AHBottomNavigation {
+    private boolean itemsCreationEnabled = true;
+    private boolean shouldCreateItems = true;
+
+    public void disableItemsCreation() {
+        itemsCreationEnabled = false;
+    }
+
+    public void enableItemsCreation() {
+        itemsCreationEnabled = true;
+        if (shouldCreateItems) createItems();
+    }
+
     public BottomTabs(Context context) {
         super(context);
         setId(CompatUtils.generateViewId());
         setContentDescription("BottomTabs");
     }
 
-    public void setTabTestId(int index, Text testId) {
-        if (!testId.hasValue() ) return;
-        perform(getViewAtPosition(index), view -> {
-            view.setTag(testId.get());
-            if (BuildConfig.DEBUG) view.setContentDescription(testId.get());
-        });
+    @Override
+    protected void createItems() {
+        if (itemsCreationEnabled) {
+            superCreateItems();
+        } else {
+            shouldCreateItems = true;
+        }
+    }
+
+    @Override
+    protected void onSizeChanged(int w, int h, int oldw, int oldh) {
+
+    }
+
+    public void superCreateItems() {
+        super.createItems();
     }
 
     public void setBadge(int bottomTabIndex, String badge) {

lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabPresenterTest.java

@@ -51,7 +51,7 @@ public class BottomTabPresenterTest extends BaseTest {
 
     @Test
     public void present() {
-        uut.present();
+        uut.applyOptions();
         for (int i = 0; i < tabs.size(); i++) {
             verify(bottomTabs, times(1)).setBadge(i, tabs.get(i).options.bottomTabOptions.badge.get(""));
             verify(bottomTabs, times(1)).setTitleInactiveColor(i, tabs.get(i).options.bottomTabOptions.textColor.get(null));

lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabsControllerTest.java

@@ -51,6 +51,7 @@ import static org.mockito.Mockito.when;
 public class BottomTabsControllerTest extends BaseTest {
 
     private Activity activity;
+    private BottomTabs bottomTabs;
     private BottomTabsController uut;
     private Options initialOptions = new Options();
     private ViewController child1;
@@ -69,6 +70,12 @@ public class BottomTabsControllerTest extends BaseTest {
     @Override
     public void beforeEach() {
         activity = newActivity();
+        bottomTabs = spy(new BottomTabs(activity) {
+            @Override
+            public void superCreateItems() {
+
+            }
+        });
         childRegistry = new ChildControllersRegistry();
         eventEmitter = Mockito.mock(EventEmitter.class);
 
@@ -97,6 +104,14 @@ public class BottomTabsControllerTest extends BaseTest {
         createBottomTabs();
     }
 
+    @Test
+    public void parentControllerIsSet() {
+        uut = createBottomTabs();
+        for (ViewController tab : tabs) {
+            assertThat(tab.getParentController()).isEqualTo(uut);
+        }
+    }
+
     @Test
     public void setTabs_allChildViewsAreAttachedToHierarchy() {
         uut.onViewAppeared();
@@ -180,6 +195,12 @@ public class BottomTabsControllerTest extends BaseTest {
         assertThat(optionsCaptor.getValue().bottomTabsOptions.backgroundColor.hasValue()).isFalse();
     }
 
+    @Test
+    public void applyOptions_bottomTabsCreateViewOnlyOnce() {
+        verify(presenter).applyOptions(any());
+        verify(bottomTabs, times(2)).superCreateItems(); // first time when view is created, second time when options are applied
+    }
+
     @Test
     public void mergeOptions_currentTabIndex() {
         uut.ensureViewIsCreated();
@@ -236,6 +257,17 @@ public class BottomTabsControllerTest extends BaseTest {
             public Options resolveCurrentOptions() {
                 return resolvedOptions;
             }
+
+            @NonNull
+            @Override
+            protected BottomTabs createBottomTabs() {
+                return new BottomTabs(activity) {
+                    @Override
+                    protected void createItems() {
+
+                    }
+                };
+            }
         };
 
         activity.setContentView(uut.getView());
@@ -347,6 +379,12 @@ public class BottomTabsControllerTest extends BaseTest {
                 uut.getView().layout(0, 0, 1000, 1000);
                 uut.getBottomTabs().layout(0, 0, 1000, 100);
             }
+
+            @NonNull
+            @Override
+            protected BottomTabs createBottomTabs() {
+                return bottomTabs;
+            }
         };
     }
 }

lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/navigator/NavigatorTest.java

@@ -31,6 +31,7 @@ import com.reactnativenavigation.viewcontrollers.ViewController;
 import com.reactnativenavigation.viewcontrollers.bottomtabs.BottomTabsController;
 import com.reactnativenavigation.viewcontrollers.modal.ModalStack;
 import com.reactnativenavigation.viewcontrollers.stack.StackController;
+import com.reactnativenavigation.views.BottomTabs;
 
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -347,7 +348,18 @@ public class NavigatorTest extends BaseTest {
 
     @NonNull
     private BottomTabsController newTabs(List<ViewController> tabs) {
-        return new BottomTabsController(activity, tabs, childRegistry, eventEmitter, imageLoaderMock, "tabsController", new Options(), new Presenter(activity, new Options()), new BottomTabsPresenter(tabs, new Options()), new BottomTabPresenter(activity, tabs, ImageLoaderMock.mock(), new Options()));
+        return new BottomTabsController(activity, tabs, childRegistry, eventEmitter, imageLoaderMock, "tabsController", new Options(), new Presenter(activity, new Options()), new BottomTabsPresenter(tabs, new Options()), new BottomTabPresenter(activity, tabs, ImageLoaderMock.mock(), new Options())) {
+            @NonNull
+            @Override
+            protected BottomTabs createBottomTabs() {
+                return new BottomTabs(activity) {
+                    @Override
+                    public void superCreateItems() {
+
+                    }
+                };
+            }
+        };
     }
 
     @Test