Skip to content

Commit

Permalink
Mount all stack children (#6194)
Browse files Browse the repository at this point in the history
This commit somewhat consolidates how stack children are mounted.

On Android, until now, only the top child in a stack was mounted when the stack became visible or when setStackRoot() was called. When popping the stack the new current child would be mounted when it was attached. This caused flickering when popping the stack as the pop animation started before the child finished mounting.

Now all children will be mounted together after the top child is mounted :)

Co-authored-by: Yogev Ben David <[email protected]>
  • Loading branch information
guyca and yogevbd authored May 18, 2020
1 parent 67191e9 commit a1beebe
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected void onAttachedToWindow() {
start();
}

private void start() {
public void start() {
if (isAttachedToReactInstance) return;
isAttachedToReactInstance = true;
final Bundle opts = new Bundle();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.reactnativenavigation.presentation.Presenter;
import com.reactnativenavigation.utils.StatusBarUtils;
import com.reactnativenavigation.views.ComponentLayout;
import com.reactnativenavigation.views.ReactComponent;

import androidx.annotation.NonNull;
import androidx.core.view.ViewCompat;
Expand All @@ -22,10 +21,6 @@ public class ComponentViewController extends ChildController<ComponentLayout> {
private ComponentPresenter presenter;
private final ReactViewCreator viewCreator;

ReactComponent getComponent() {
return view;
}

public ComponentViewController(final Activity activity,
final ChildControllersRegistry childRegistry,
final String id,
Expand All @@ -40,6 +35,11 @@ public ComponentViewController(final Activity activity,
this.presenter = componentPresenter;
}

@Override
public void start() {
if (!isDestroyed()) getView().start();
}

@Override
public String getCurrentComponentName() {
return this.componentName;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
package com.reactnativenavigation.viewcontrollers;

import androidx.annotation.NonNull;

import com.reactnativenavigation.utils.StringUtils;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import static com.reactnativenavigation.utils.CollectionUtils.last;
import static com.reactnativenavigation.utils.CollectionUtils.removeLast;
import androidx.annotation.NonNull;

import static com.reactnativenavigation.utils.CollectionUtils.*;

public class IdStack<E> implements Iterable<String> {

Expand Down Expand Up @@ -84,8 +83,8 @@ public Iterator<String> iterator() {
}


public Collection<E> values() {
return map.values();
public List<E> values() {
return map(deque, map::get);
}

public void remove(Iterator<String> iterator, String id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ public boolean isRendered() {
);
}

public void start() {

}

void applyOnController(ViewController controller, Func1<ViewController> task) {
if (controller != null) task.run(controller);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.reactnativenavigation.views.stack.StackBehaviour;
import com.reactnativenavigation.views.topbar.TopBar;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -214,6 +215,7 @@ public void onSuccess(String childId) {
backButtonHelper.addToPushedChild(children.get(i));
}
}
startChildrenBellowTopChild();
}
listener.onSuccess(childId);
}
Expand Down Expand Up @@ -360,10 +362,18 @@ private void addInitialChild(StackLayout stackLayout) {
if (isEmpty()) return;
ViewGroup child = peek().getView();
child.setId(CompatUtils.generateViewId());
peek().addOnAppearedListener(this::startChildrenBellowTopChild);
presenter.applyInitialChildLayoutOptions(resolveCurrentOptions());
stackLayout.addView(child, 0, matchParentWithBehaviour(new StackBehaviour(this)));
}

private void startChildrenBellowTopChild() {
ArrayList<ViewController> children = new ArrayList(getChildControllers());
for (int i = children.size() - 2; i >= 0; i--) {
children.get(i).start();
}
}

private void onNavigationButtonPressed(String buttonId) {
if (Constants.BACK_BUTTON_ID.equals(buttonId)) {
pop(Options.EMPTY, new CommandListenerAdapter());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public void destroy() {
reactView.destroy();
}

public void start() {
reactView.start();
}

public void sendComponentStart() {
reactView.sendComponentStart(ComponentType.Component);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,6 @@ public void values() {
assertThat(uut.values()).isNotNull().isEmpty();
uut.push("123", 123);
uut.push("456", 456);
assertThat(uut.values()).isNotNull().containsExactlyInAnyOrder(123, 456);
assertThat(uut.values()).isNotNull().containsSequence(123, 456);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
import com.reactnativenavigation.anim.NavigationAnimator;
import com.reactnativenavigation.mocks.ImageLoaderMock;
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.AnimationOptions;
import com.reactnativenavigation.parse.NestedAnimationsOptions;
import com.reactnativenavigation.parse.Options;
Expand Down Expand Up @@ -59,6 +59,7 @@

import androidx.coordinatorlayout.widget.CoordinatorLayout;

import static com.reactnativenavigation.utils.ObjectUtils.take;
import static com.reactnativenavigation.utils.ViewUtils.topMargin;
import static org.assertj.core.api.Java6Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
Expand All @@ -82,6 +83,7 @@ public class StackControllerTest extends BaseTest {
private ViewController child1a;
private ViewController child2;
private ViewController child3;
private SimpleViewController.SimpleView child3View;
private ViewController child4;
private NavigationAnimator animator;
private TopBarController topBarController;
Expand All @@ -108,13 +110,22 @@ public void beforeEach() {
new Options()
)
);
createChildren();
uut = createStack();
activity.setContentView(uut.getView());
}

private void createChildren() {
child1 = spy(new SimpleViewController(activity, childRegistry, "child1", new Options()));
child1a = spy(new SimpleViewController(activity, childRegistry, "child1", new Options()));
child2 = spy(new SimpleViewController(activity, childRegistry, "child2", new Options()));
child3 = spy(new SimpleViewController(activity, childRegistry, "child3", new Options()));
child3 = spy(new SimpleViewController(activity, childRegistry, "child3", new Options()) {
@Override
protected SimpleView createView() {
return take(child3View, super.createView());
}
});
child4 = spy(new SimpleViewController(activity, childRegistry, "child4", new Options()));
uut = createStack();
activity.setContentView(uut.getView());
}

@Test
Expand Down Expand Up @@ -394,6 +405,19 @@ public void setRoot_doesNotCrashWhenCalledWithSameId() {
assertContainsOnlyId(child1a.getId());
}

@Test
public void setRoot_topScreenIsStartedThenTheRest() {
disablePushAnimation(child1, child2, child3);
child3View = spy(new SimpleViewController.SimpleView(activity));

uut.setRoot(Arrays.asList(child1, child2, child3), new CommandListenerAdapter());
ShadowLooper.idleMainLooper();
InOrder inOrder = inOrder(child3View, child2, child1);
inOrder.verify(child3View).start();
inOrder.verify(child2).start();
inOrder.verify(child1).start();
}

@Test
public synchronized void pop() {
disablePushAnimation(child1, child2);
Expand Down

0 comments on commit a1beebe

Please sign in to comment.