From b84a3e5fadcbef93a8ef683550743dc84ac8a2fa Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Wed, 19 Dec 2018 18:02:16 +0200 Subject: [PATCH] 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 | 2 +- .../presentation/BottomTabPresenter.java | 2 +- .../presentation/BottomTabsPresenter.java | 2 +- .../utils/ImageLoader.java | 10 +++ .../com/reactnativenavigation/utils/Time.java | 2 +- .../bottomtabs/BottomTabsController.java | 70 +++++++------------ .../views/BottomTabs.java | 38 +++++++--- .../BottomTabPresenterTest.java | 2 +- .../BottomTabsControllerTest.java | 38 ++++++++++ .../navigator/NavigatorTest.java | 14 +++- 10 files changed, 119 insertions(+), 61 deletions(-) diff --git a/lib/android/app/build.gradle b/lib/android/app/build.gradle index 96790f28de1..7367c5258d2 100644 --- a/lib/android/app/build.gradle +++ b/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' diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabPresenter.java b/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabPresenter.java index 2d6eca0479f..e9950930f7a 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabPresenter.java +++ b/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabPresenter.java @@ -45,7 +45,7 @@ public void bindView(BottomTabs bottomTabs) { 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("")); diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabsPresenter.java b/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabsPresenter.java index e67b5ad3549..8dc724332e5 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabsPresenter.java +++ b/lib/android/app/src/main/java/com/reactnativenavigation/presentation/BottomTabsPresenter.java @@ -52,7 +52,7 @@ public void mergeOptions(Options options) { 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); } diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/utils/ImageLoader.java b/lib/android/app/src/main/java/com/reactnativenavigation/utils/ImageLoader.java index a07175d8379..d61f9c1d156 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/utils/ImageLoader.java +++ b/lib/android/app/src/main/java/com/reactnativenavigation/utils/ImageLoader.java @@ -33,6 +33,16 @@ public interface ImagesLoadingListener { 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)); diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/utils/Time.java b/lib/android/app/src/main/java/com/reactnativenavigation/utils/Time.java index 6467d199b5d..e72a2fc54de 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/utils/Time.java +++ b/lib/android/app/src/main/java/com/reactnativenavigation/utils/Time.java @@ -29,7 +29,7 @@ public static void log(String tag, Functions.Func task) { 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"); } diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/bottomtabs/BottomTabsController.java b/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/bottomtabs/BottomTabsController.java index 5e4a46ea3bd..b0b37b5bff9 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/bottomtabs/BottomTabsController.java +++ b/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.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 BottomTabsController(Activity activity, List tabs, ChildC this.imageLoader = imageLoader; this.presenter = bottomTabsPresenter; this.tabPresenter = bottomTabPresenter; + forEach(tabs, (tab) -> tab.setParentController(this)); } @Override @@ -62,22 +62,30 @@ public void setDefaultOptions(Options defaultOptions) { @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 boolean onTabSelected(int index, boolean wasSelected) { return false; } - private void createTabs(RelativeLayout root) { - if (tabs.size() > 5) { - throw new RuntimeException("Too many tabs!"); - } - List icons = new ArrayList<>(); - List 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 drawables) { - List 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 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("") + ); }); } diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/views/BottomTabs.java b/lib/android/app/src/main/java/com/reactnativenavigation/views/BottomTabs.java index e64f22532b0..eb2d9b74981 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/views/BottomTabs.java +++ b/lib/android/app/src/main/java/com/reactnativenavigation/views/BottomTabs.java @@ -8,26 +8,44 @@ 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) { diff --git a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabPresenterTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabPresenterTest.java index b1902744caf..d687ee14f74 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabPresenterTest.java +++ b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabPresenterTest.java @@ -51,7 +51,7 @@ public void beforeEach() { @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)); diff --git a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabsControllerTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabsControllerTest.java index 37ef3e33a74..b992bb8c800 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabsControllerTest.java +++ b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/BottomTabsControllerTest.java @@ -51,6 +51,7 @@ 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 void setTabs_ThrowWhenMoreThan5() { 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 void applyOptions_bottomTabsOptionsAreClearedAfterApply() { 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 void applyChildOptions_resolvedOptionsAreUsed() { 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 void ensureViewIsCreated() { uut.getView().layout(0, 0, 1000, 1000); uut.getBottomTabs().layout(0, 0, 1000, 100); } + + @NonNull + @Override + protected BottomTabs createBottomTabs() { + return bottomTabs; + } }; } } diff --git a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/navigator/NavigatorTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/navigator/NavigatorTest.java index b632427eba1..2cb83004caa 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/navigator/NavigatorTest.java +++ b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/navigator/NavigatorTest.java @@ -31,6 +31,7 @@ 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 void mergeOptions_AffectsOnlyComponentViewControllers() { @NonNull private BottomTabsController newTabs(List 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