Skip to content

Commit

Permalink
Fix closing sideMenu when pushing a screen (#4491)
Browse files Browse the repository at this point in the history 
When pushing a screen into a stack in the centre controller with `sideMenu.left/right.visible: false` - side menu was not closed as expected.
This happened because when either side menus is open, it’s considered the current child and when options are resolved. the centre controller’s options were ignored.

This commit fixes this issue on Android. When resolving options, the centre controllers options are resolved as well.
Related to #4267
  • Loading branch information
@guyca
guyca authored Dec 23, 2018
1 parent 13de5ca commit dc739de
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 54 deletions.
23 changes: 11 additions & 12 deletions lib/android/app/src/main/java/com/reactnativenavigation/parse/SideMenuOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,16 @@ public static SideMenuOptions parse(JSONObject json) {
}

public void mergeWith(SideMenuOptions other) {
if (other.visible.hasValue()) {
visible = other.visible;
}
if (other.enabled.hasValue()) {
enabled = other.enabled;
}
if (other.height.hasValue()) {
height = other.height;
}
if (other.width.hasValue()) {
width = other.width;
}
if (other.visible.hasValue()) visible = other.visible;
if (other.enabled.hasValue()) enabled = other.enabled;
if (other.height.hasValue()) height = other.height;
if (other.width.hasValue()) width = other.width;
}

public void mergeWithDefault(SideMenuOptions defaultOptions) {
if (!visible.hasValue()) visible = defaultOptions.visible;
if (!enabled.hasValue()) enabled = defaultOptions.enabled;
if (!height.hasValue()) height = defaultOptions.height;
if (!width.hasValue()) width = defaultOptions.width;
}
}
75 changes: 43 additions & 32 deletions lib/android/app/src/main/java/com/reactnativenavigation/presentation/SideMenuPresenter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.support.v4.widget.DrawerLayout;
import android.view.Gravity;

import com.reactnativenavigation.parse.Options;
import com.reactnativenavigation.parse.SideMenuRootOptions;

public class SideMenuPresenter {
Expand All @@ -13,56 +14,66 @@ public void bindView(DrawerLayout sideMenu) {
this.sideMenu = sideMenu;
}

/**
* Called when initializing the sideMenu DrawerLayout.
*
* @param options Side menu options
*/
public void applyInitialOptions(SideMenuRootOptions options) {
if (options.left.enabled.isFalse()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED, Gravity.LEFT);
public boolean handleBack() {
if (sideMenu.isDrawerOpen(Gravity.LEFT)) {
sideMenu.closeDrawer(Gravity.LEFT);
return true;
}
else if (options.left.enabled.isTrue()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_UNLOCKED, Gravity.LEFT);
if (sideMenu.isDrawerOpen(Gravity.RIGHT)) {
sideMenu.closeDrawer(Gravity.RIGHT);
return true;
}
return false;
}

if (options.right.enabled.isFalse()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED, Gravity.RIGHT);
}
else if (options.right.enabled.isTrue()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_UNLOCKED, Gravity.RIGHT);
}
public void mergeOptions(SideMenuRootOptions options) {
mergeLockMode(options);
mergeVisibility(options);
}

public void mergeChildOptions(SideMenuRootOptions options) {
mergeLockMode(options);
mergeVisibility(options);
}

public void applyChildOptions(Options options) {
applyLockMode(options.sideMenuRootOptions);
mergeVisibility(options.sideMenuRootOptions);
}

public void present(SideMenuRootOptions options) {
// TODO: Not sure why we call these options when we show the DrawerLayout rather than when initializing it.
// TODO: (i.e. `setDrawerLockMode()` is supposed to be called when the DrawerLayout is initialized.
applyInitialOptions(options);
private void applyLockMode(SideMenuRootOptions options) {
int leftLockMode = options.left.enabled.isTrueOrUndefined() ? DrawerLayout.LOCK_MODE_UNLOCKED : DrawerLayout.LOCK_MODE_LOCKED_CLOSED;
sideMenu.setDrawerLockMode(leftLockMode, Gravity.LEFT);

int rightLockMode = options.right.enabled.isTrueOrUndefined() ? DrawerLayout.LOCK_MODE_UNLOCKED : DrawerLayout.LOCK_MODE_LOCKED_CLOSED;
sideMenu.setDrawerLockMode(rightLockMode, Gravity.RIGHT);
}

private void mergeVisibility(SideMenuRootOptions options) {
if (options.left.visible.isTrue()) {
sideMenu.openDrawer(Gravity.LEFT);

} else if (options.left.visible.isFalse() && sideMenu.isDrawerOpen(Gravity.LEFT)) {
} else if (options.left.visible.isFalse()) {
sideMenu.closeDrawer(Gravity.LEFT);
}

if (options.right.visible.isTrue()) {
sideMenu.openDrawer(Gravity.RIGHT);

} else if (options.right.visible.isFalse() && sideMenu.isDrawerOpen(Gravity.RIGHT)){
} else if (options.right.visible.isFalse()) {
sideMenu.closeDrawer(Gravity.RIGHT);
}
}

public boolean handleBack() {
if (sideMenu.isDrawerOpen(Gravity.LEFT)) {
sideMenu.closeDrawer(Gravity.LEFT);
return true;
private void mergeLockMode(SideMenuRootOptions options) {
if (options.left.enabled.isFalse()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED, Gravity.LEFT);
} else if (options.left.enabled.isTrue()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_UNLOCKED, Gravity.LEFT);
}
if (sideMenu.isDrawerOpen(Gravity.RIGHT)) {
sideMenu.closeDrawer(Gravity.RIGHT);
return true;

if (options.right.enabled.isFalse()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED, Gravity.RIGHT);
} else if (options.right.enabled.isTrue()) {
sideMenu.setDrawerLockMode(DrawerLayout.LOCK_MODE_UNLOCKED, Gravity.RIGHT);
}
return false;
}
}
15 changes: 12 additions & 3 deletions .../src/main/java/com/reactnativenavigation/viewcontrollers/sidemenu/SideMenuController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public Collection<ViewController> getChildControllers() {
@Override
public void applyChildOptions(Options options, Component child) {
super.applyChildOptions(options, child);
presenter.applyInitialOptions(options.sideMenuRootOptions);
presenter.applyChildOptions(resolveCurrentOptions());
performOnParentController(parentController ->
((ParentController) parentController).applyChildOptions(this.options, child)
);
Expand All @@ -81,7 +81,7 @@ public void applyChildOptions(Options options, Component child) {
@Override
public void mergeChildOptions(Options options, ViewController childController, Component child) {
super.mergeChildOptions(options, childController, child);
presenter.present(options.sideMenuRootOptions);
presenter.mergeChildOptions(options.sideMenuRootOptions);
performOnParentController(parentController ->
((ParentController) parentController).mergeChildOptions(options.copy().clearSideMenuOptions(), childController, child)
);
Expand All @@ -90,7 +90,16 @@ public void mergeChildOptions(Options options, ViewController childController, C
@Override
public void mergeOptions(Options options) {
super.mergeOptions(options);
presenter.present(this.options.sideMenuRootOptions);
presenter.mergeOptions(options.sideMenuRootOptions);
}

@Override
public Options resolveCurrentOptions() {
Options options = super.resolveCurrentOptions();
if (getView().isDrawerOpen(Gravity.LEFT) || getView().isDrawerOpen(Gravity.RIGHT)) {
options = options.mergeWith(center.resolveCurrentOptions());
}
return options;
}

@Override
Expand Down
53 changes: 49 additions & 4 deletions .../test/java/com/reactnativenavigation/viewcontrollers/sidemenu/SideMenuControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@
import com.reactnativenavigation.parse.SideMenuOptions;
import com.reactnativenavigation.parse.params.Bool;
import com.reactnativenavigation.parse.params.Number;
import com.reactnativenavigation.parse.params.Text;
import com.reactnativenavigation.presentation.Presenter;
import com.reactnativenavigation.presentation.SideMenuPresenter;
import com.reactnativenavigation.utils.CommandListenerAdapter;
import com.reactnativenavigation.viewcontrollers.ChildControllersRegistry;
import com.reactnativenavigation.viewcontrollers.ParentController;
import com.reactnativenavigation.viewcontrollers.ViewController;
import com.reactnativenavigation.views.Component;

import org.junit.Test;
import org.mockito.Mockito;

import static android.view.ViewGroup.LayoutParams.MATCH_PARENT;
import static org.assertj.core.api.Java6Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand All @@ -32,20 +38,32 @@ public class SideMenuControllerTest extends BaseTest {
private Activity activity;
private ChildControllersRegistry childRegistry;
private SideMenuPresenter presenter;
private SimpleComponentViewController left;
private SimpleComponentViewController right;
private SimpleComponentViewController center;
private ViewController left;
private ViewController right;
private ViewController center;
private ViewController child;
private ParentController parent;
private Options resolvedOptions;

@Override
public void beforeEach() {
activity = newActivity();
childRegistry = new ChildControllersRegistry();
presenter = spy(new SideMenuPresenter());
child = new SimpleComponentViewController(activity, childRegistry, "child", new Options());
left = new SimpleComponentViewController(activity, childRegistry, "left", new Options());
right = new SimpleComponentViewController(activity, childRegistry, "right", new Options());
center = spy(new SimpleComponentViewController(activity, childRegistry, "center", new Options()));
uut = new SideMenuController(activity, childRegistry, "sideMenu", new Options(), presenter, new Presenter(activity, new Options()));
uut = new SideMenuController(activity, childRegistry, "sideMenu", new Options(), presenter, new Presenter(activity, new Options())) {
@Override
public Options resolveCurrentOptions() {
resolvedOptions = super.resolveCurrentOptions();
return resolvedOptions;
}
};
uut.setCenterController(center);
parent = Mockito.mock(ParentController.class);
uut.setParentController(parent);
}

@Test
Expand All @@ -54,6 +72,13 @@ public void createView_bindView() {
verify(presenter).bindView(uut.getView());
}

@Test
public void applyChildOptions() {
uut.applyChildOptions(new Options(), (Component) child.getView());
verify(presenter).applyChildOptions(eq(resolvedOptions));
verify(parent).applyChildOptions(uut.options, (Component) child.getView());
}

@Test
public void mergeOptions_openLeftSideMenu() {
uut.setLeftController(new SimpleComponentViewController(activity, childRegistry, "left", new Options()));
Expand Down Expand Up @@ -84,6 +109,23 @@ public void mergeOptions_optionsAreClearedAfterMerge() {
assertThat(uut.options.sideMenuRootOptions).isNotEqualTo(initialOptions.sideMenuRootOptions);
}

@Test
public void mergeChildOptions() {
Options options = new Options();
uut.mergeChildOptions(options, child, (Component) child.getView());
verify(presenter).mergeChildOptions(options.sideMenuRootOptions);
}

@Test
public void resolveCurrentOptions_centerOptionsAreMergedEvenIfDrawerIsOpen() {
uut.setLeftController(left);
center.options.topBar.title.text = new Text("Center");
assertThat(uut.resolveCurrentOptions().topBar.title.text.hasValue()).isTrue();

uut.getView().openDrawer(Gravity.LEFT);
assertThat(uut.resolveCurrentOptions().topBar.title.text.hasValue()).isTrue();
}

@Test
public void setLeftController_matchesParentByDefault() {
SideMenuOptions options = new SideMenuOptions();
Expand All @@ -98,6 +140,7 @@ public void setLeftController_matchesParentByDefault() {
assertThat(params.width).isEqualTo(MATCH_PARENT);
assertThat(params.height).isEqualTo(MATCH_PARENT);
}

@Test
public void setLeftController_setHeightAndWidthWithOptions() {
SideMenuOptions options = new SideMenuOptions();
Expand All @@ -115,6 +158,7 @@ public void setLeftController_setHeightAndWidthWithOptions() {
assertThat(params.width).isEqualTo(widthInDp);
assertThat(params.height).isEqualTo(heightInDp);
}

@Test
public void setRightController_matchesParentByDefault() {
SideMenuOptions options = new SideMenuOptions();
Expand All @@ -129,6 +173,7 @@ public void setRightController_matchesParentByDefault() {
assertThat(params.width).isEqualTo(MATCH_PARENT);
assertThat(params.height).isEqualTo(MATCH_PARENT);
}

@Test
public void setRightController_setHeightAndWidthWithOptions() {
SideMenuOptions options = new SideMenuOptions();
Expand Down
16 changes: 13 additions & 3 deletions playground/src/screens/SideMenuScreen.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const React = require('react');
const { Component } = require('react');

const { View, Text, Button } = require('react-native');
const { View, Text, Button, Platform } = require('react-native');

const { Navigation } = require('react-native-navigation');
const testIDs = require('../testIDs');
Expand Down Expand Up @@ -31,10 +31,20 @@ class SideMenuScreen extends Component {
}

pushAndCloseSideMenu() {
this.hideSideMenu();
if (Platform.OS === 'ios') {
this.hideSideMenu();
}
Navigation.push('tab1Stack', {
component: {
name: 'navigation.playground.TextScreen'
name: 'navigation.playground.TextScreen',
options: {
sideMenu: {
left: {
visible: false,
enabled: false
}
}
}
}
});
}
Expand Down

0 comments on commit dc739de

Please sign in to comment.