Skip to content

Commit

Permalink
Stack declared children crash on duplicate id (#7074)
Browse files Browse the repository at this point in the history
# Issue
Calling setRoot with children that have the same ids,  caused pop to crash, not aligned with `Navigation.push` behaviour
```javascript
  setRoot = async () =>
    await Navigation.setRoot({
      root: {
        stack: {
          id: 'stack',
          children: [
            {
              component: {
                id: 'component',
                name: Screens.A,
              },
            },
              {
              component: {
                id: 'component',
                name: Screens.A,
              },
            },
          ],
        },
      },
    });

```

# Fix

Since such behaviour cannot be ignored, pop will always fail if we just ignore the ids, the fix is to be aligned with `push` command.
  • Loading branch information
swabbass authored Apr 7, 2021
1 parent 3f4c554 commit ea1088d
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public void sendOnNavigationButtonPressed(String buttonId) {
}

@Override
public ViewController getCurrentChild() {
public ViewController<?> getCurrentChild() {
return tabs.get(bottomTabs == null ? 0 : bottomTabs.getCurrentItem());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ public StackController(Activity activity, List<ViewController> children, ChildCo
this.fabPresenter = fabPresenter;
stackPresenter.setButtonOnClickListener(this::onNavigationButtonPressed);
for (ViewController child : children) {
if (stack.containsId(child.getId())) {
throw new IllegalArgumentException("A stack can't contain two children with the same id: " + child.getId());
}
child.setParentController(this);
stack.push(child.getId(), child);
if (size() > 1) backButtonHelper.addToPushedChild(child);
Expand All @@ -86,7 +89,7 @@ public void setDefaultOptions(Options defaultOptions) {
}

@Override
public ViewController getCurrentChild() {
public ViewController<?> getCurrentChild() {
return stack.peek();
}

Expand Down Expand Up @@ -151,7 +154,7 @@ public void onChildDestroyed(ViewController child) {

public void push(ViewController child, CommandListener listener) {
if (findController(child.getId()) != null) {
listener.onError("A stack can't contain two children with the same id");
listener.onError("A stack can't contain two children with the same id: " + child.getId());
return;
}
final ViewController toRemove = stack.peek();
Expand Down Expand Up @@ -240,7 +243,7 @@ public void onSuccess(String childId) {
resolvedOptions,
presenter.getAdditionalSetRootAnimations(this, child, resolvedOptions),
() -> listenerAdapter.onSuccess(child.getId())
)
)
);
} else {
animator.setRoot(child,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ import org.json.JSONObject
import org.junit.Ignore
import org.junit.Test
import org.robolectric.Robolectric
import org.robolectric.annotation.LooperMode
import org.robolectric.shadows.ShadowLooper
import java.util.*
import kotlin.jvm.Throws
import kotlin.test.fail

class StackControllerTest : BaseTest() {
private lateinit var activity: Activity
Expand Down Expand Up @@ -99,6 +98,16 @@ class StackControllerTest : BaseTest() {
assertThat(uut).isInstanceOf(ViewController::class.java)
}

@Test
fun childrenMustBeUniqueById() {
try {
val uut: StackController = createStack(listOf(child1, child2, child1))
fail("Stack should not have duplicate ids!")
} catch (e: IllegalArgumentException) {
assertThat(e.message).contains(child1.id)
}
}

@Test
fun childrenAreAssignedParent() {
val uut: StackController = createStack(listOf(child1, child2))
Expand Down

0 comments on commit ea1088d

Please sign in to comment.