Skip to content

Commit

Permalink
fix(menu): open/close race condition
Browse files Browse the repository at this point in the history 
fixes ionic-team#7629 ionic-team#8001
  • Loading branch information
@manucorporat
manucorporat committed Sep 14, 2016
1 parent 6506cd5 commit 5fabba2
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 93 deletions.
19 changes: 12 additions & 7 deletions src/components/menu/menu-gestures.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class MenuContentGesture extends SlideEdgeGesture {

canStart(ev: any): boolean {
let menu = this.menu;
if (!menu.enabled || !menu.swipeEnabled) {
if (!menu.canSwipe()) {
return false;
}
if (menu.isOpen) {
Expand All @@ -36,15 +36,23 @@ export class MenuContentGesture extends SlideEdgeGesture {
}

// Set CSS, then wait one frame for it to apply before sliding starts
onSlideBeforeStart(slide: SlideData, ev: any) {
onSlideBeforeStart(ev: any) {
console.debug('menu gesture, onSlideBeforeStart', this.menu.side);
this.menu.swipeStart();
}

onSlide(slide: SlideData, ev: any) {
let z = (this.menu.side === 'right' ? slide.min : slide.max);
let stepValue = (slide.distance / z);
console.debug('menu gesture, onSlide', this.menu.side, 'distance', slide.distance, 'min', slide.min, 'max', slide.max, 'z', z, 'stepValue', stepValue);

console.debug(
'menu gesture, onSlide', this.menu.side,
'distance', slide.distance,
'min', slide.min,
'max', slide.max,
'z', z,
'stepValue', stepValue);

ev.preventDefault();
this.menu.swipeProgress(stepValue);
}
Expand All @@ -70,28 +78,25 @@ export class MenuContentGesture extends SlideEdgeGesture {
'shouldCompleteLeft', shouldCompleteLeft,
'shouldCompleteRight', shouldCompleteRight,
'currentStepValue', currentStepValue);

this.menu.swipeEnd(shouldCompleteLeft, shouldCompleteRight, currentStepValue);
}

getElementStartPos(slide: SlideData, ev: any) {
if (this.menu.side === 'right') {
// right menu
return this.menu.isOpen ? slide.min : slide.max;
}

// left menu
return this.menu.isOpen ? slide.max : slide.min;
}

getSlideBoundaries(): {min: number, max: number} {
if (this.menu.side === 'right') {
// right menu
return {
min: -this.menu.width(),
max: 0
};
}

// left menu
return {
min: 0,
Expand Down
132 changes: 66 additions & 66 deletions src/components/menu/menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,17 @@ import { GestureController } from '../../gestures/gesture-controller';
encapsulation: ViewEncapsulation.None,
})
export class Menu {
private _preventTime: number = 0;
private _cntEle: HTMLElement;
private _cntGesture: MenuContentGesture;
private _type: MenuType;
private _resizeUnreg: Function;
private _isEnabled: boolean = true;
private _isSwipeEnabled: boolean = true;
private _isAnimating: boolean = false;
private _isPers: boolean = false;
private _init: boolean = false;


/**
* @private
*/
Expand Down Expand Up @@ -378,21 +379,20 @@ export class Menu {
* @private
*/
private _setListeners() {
let self = this;

if (self._init) {
// only listen/unlisten if the menu has initialized
if (!this._init) {
return;
}

if (self._isEnabled && self._isSwipeEnabled && !self._cntGesture.isListening) {
// should listen, but is not currently listening
console.debug('menu, gesture listen', self.side);
self._cntGesture.listen();
// only listen/unlisten if the menu has initialized
if (this._isEnabled && this._isSwipeEnabled && !this._cntGesture.isListening) {
// should listen, but is not currently listening
console.debug('menu, gesture listen', this.side);
this._cntGesture.listen();

} else if (self._cntGesture.isListening && (!self._isEnabled || !self._isSwipeEnabled)) {
// should not listen, but is currently listening
console.debug('menu, gesture unlisten', self.side);
self._cntGesture.unlisten();
}
} else if (this._cntGesture.isListening && (!this._isEnabled || !this._isSwipeEnabled)) {
// should not listen, but is currently listening
console.debug('menu, gesture unlisten', this.side);
this._cntGesture.unlisten();
}
}

Expand All @@ -416,7 +416,7 @@ export class Menu {
setOpen(shouldOpen: boolean, animated: boolean = true): Promise<boolean> {
// _isPrevented is used to prevent unwanted opening/closing after swiping open/close
// or swiping open the menu while pressing down on the MenuToggle button
if ((shouldOpen && this.isOpen) || this._isPrevented()) {
if ((shouldOpen && this.isOpen) || !this._isEnabled || this._isAnimating) {
return Promise.resolve(this.isOpen);
}

Expand All @@ -430,12 +430,20 @@ export class Menu {
});
}


/**
* @private
*/
canSwipe(): boolean {
return this._isEnabled && this._isSwipeEnabled && !this._isAnimating;
}

/**
* @private
*/
swipeStart() {
// user started swiping the menu open/close
if (this._isEnabled && this._isSwipeEnabled && !this._isPrevented()) {
if (this.canSwipe()) {
this._before();
this._getType().setProgressStart(this.isOpen);
}
Expand All @@ -446,84 +454,67 @@ export class Menu {
*/
swipeProgress(stepValue: number) {
// user actively dragging the menu
if (this._isEnabled && this._isSwipeEnabled) {
this._prevent();
this._getType().setProgessStep(stepValue);
this.ionDrag.emit(stepValue);
if (!this._isAnimating) {
return;
}
this._getType().setProgessStep(stepValue);
this.ionDrag.emit(stepValue);
}

/**
* @private
*/
swipeEnd(shouldCompleteLeft: boolean, shouldCompleteRight: boolean, stepValue: number) {
if (!this._isAnimating) {
return;
}
// user has finished dragging the menu
if (this._isEnabled && this._isSwipeEnabled) {
this._prevent();

let opening = !this.isOpen;
let shouldComplete = false;
if (opening) {
shouldComplete = (this.side === 'right') ? shouldCompleteLeft : shouldCompleteRight;
} else {
shouldComplete = (this.side === 'right') ? shouldCompleteRight : shouldCompleteLeft;
}

this._getType().setProgressEnd(shouldComplete, stepValue, (isOpen: boolean) => {
console.debug('menu, swipeEnd', this.side);
this._after(isOpen);
});
let opening = !this.isOpen;
let shouldComplete = false;
if (opening) {
shouldComplete = (this.side === 'right') ? shouldCompleteLeft : shouldCompleteRight;
} else {
shouldComplete = (this.side === 'right') ? shouldCompleteRight : shouldCompleteLeft;
}

this._getType().setProgressEnd(shouldComplete, stepValue, (isOpen: boolean) => {
console.debug('menu, swipeEnd', this.side);
this._after(isOpen);
});
}

private _before() {
// this places the menu into the correct location before it animates in
// this css class doesn't actually kick off any animations
if (this._isEnabled) {
this.getNativeElement().classList.add('show-menu');
this.getBackdropElement().classList.add('show-backdrop');

this._prevent();
this._keyboard.close();
}
this.getNativeElement().classList.add('show-menu');
this.getBackdropElement().classList.add('show-backdrop');
this._keyboard.close();
this._isAnimating = true;
}

private _after(isOpen: boolean) {
// keep opening/closing the menu disabled for a touch more yet
// only add listeners/css if it's enabled and isOpen
// and only remove listeners/css if it's not open
// emit opened/closed events
if ((this._isEnabled && isOpen) || !isOpen) {
this._prevent();

this.isOpen = isOpen;
this.isOpen = isOpen;
this._isAnimating = false;

(<any>this._cntEle.classList)[isOpen ? 'add' : 'remove']('menu-content-open');
(<any>this._cntEle.classList)[isOpen ? 'add' : 'remove']('menu-content-open');

this._cntEle.removeEventListener('click', this.onContentClick);
this._cntEle.removeEventListener('click', this.onContentClick);

if (isOpen) {
this._cntEle.addEventListener('click', this.onContentClick);
this.ionOpen.emit(true);
if (isOpen) {
this._cntEle.addEventListener('click', this.onContentClick);
this.ionOpen.emit(true);

} else {
this.getNativeElement().classList.remove('show-menu');
this.getBackdropElement().classList.remove('show-backdrop');
this.ionClose.emit(true);
}
} else {
this.getNativeElement().classList.remove('show-menu');
this.getBackdropElement().classList.remove('show-backdrop');
this.ionClose.emit(true);
}
}

private _prevent() {
// used to prevent unwanted opening/closing after swiping open/close
// or swiping open the menu while pressing down on the MenuToggle
this._preventTime = Date.now() + 20;
}

private _isPrevented() {
return this._preventTime > Date.now();
}

/**
* @private
*/
Expand Down Expand Up @@ -564,6 +555,9 @@ export class Menu {
.map(m => m.enabled = false);
}

// TODO
// what happens if menu is disabled while swipping?

return this;
}

Expand All @@ -572,6 +566,8 @@ export class Menu {
*/
swipeEnable(shouldEnable: boolean): Menu {
this.swipeEnabled = shouldEnable;
// TODO
// what happens if menu swipe is disabled while swipping?
return this;
}

Expand Down Expand Up @@ -619,7 +615,11 @@ export class Menu {
this._cntGesture && this._cntGesture.destroy();
this._type && this._type.destroy();
this._resizeUnreg && this._resizeUnreg();

this._cntGesture = null;
this._type = null;
this._cntEle = null;
this._resizeUnreg = null;
}

}
47 changes: 28 additions & 19 deletions src/gestures/slide-gesture.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,32 @@ export class SlideGesture extends PanGesture {
}

onDragStart(ev: any) {
this.slide = {};
this.onSlideBeforeStart(this.slide, ev);
this.onSlideBeforeStart(ev);
let coord = <any>pointerCoord(ev);
let pos = coord[this.direction];

this.slide = {
min: 0,
max: 0,
pointerStartPos: pos,
pos: pos,
timestamp: Date.now(),
elementStartPos: 0,
started: true,
delta: 0,
distance: 0,
velocity: 0,
};
let {min, max} = this.getSlideBoundaries(this.slide, ev);
let coord = <any>pointerCoord(ev);
this.slide.min = min;
this.slide.max = max;
this.slide.elementStartPos = this.getElementStartPos(this.slide, ev);
this.slide.pos = this.slide.pointerStartPos = coord[this.direction];
this.slide.timestamp = Date.now();
this.slide.started = true;
this.slide.velocity = 0;

this.onSlideStart(this.slide, ev);
}

onDragMove(ev: any) {
let slide = this.slide;
let slide: SlideData = this.slide;
let coord = <any>pointerCoord(ev);
let newPos = coord[this.direction];
let newTimestamp = Date.now();
Expand All @@ -74,7 +83,7 @@ export class SlideGesture extends PanGesture {
this.slide = null;
}

onSlideBeforeStart(slide?: SlideData, ev?: any): void {}
onSlideBeforeStart(ev?: any): void {}
onSlideStart(slide?: SlideData, ev?: any): void {}
onSlide(slide?: SlideData, ev?: any): void {}
onSlideEnd(slide?: SlideData, ev?: any): void {}
Expand All @@ -84,14 +93,14 @@ export class SlideGesture extends PanGesture {
* @private
*/
export interface SlideData {
min?: number;
max?: number;
distance?: number;
delta?: number;
started?: boolean;
pos?: any;
timestamp?: number;
pointerStartPos?: number;
elementStartPos?: number;
velocity?: number;
min: number;
max: number;
distance: number;
delta: number;
started: boolean;
pos: any;
timestamp: number;
pointerStartPos: number;
elementStartPos: number;
velocity: number;
}
2 changes: 1 addition & 1 deletion src/navigation/swipe-back.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class SwipeBackGesture extends SlideEdgeGesture {
}


onSlideBeforeStart(slideData: SlideData, ev: any) {
onSlideBeforeStart(ev: any) {
console.debug('swipeBack, onSlideBeforeStart', ev.type);
this._nav.swipeBackStart();
}
Expand Down

0 comments on commit 5fabba2

Please sign in to comment.