mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2026-03-13 10:22:08 +08:00
fix($ionicActionSheet): stop memory leak due to hidden element staying in dom
BREAKING CHANGE: $ionicActionSheet now returns a method to hide the
action sheet.
Previously, it returned an object that had a `show` and `hide` method.
This was undocumented, but if you used it, here is how to migrate your
code:
Change your code from this:
```js
var sheet = $ionicActionSheet.show({...});
sheet.hide();
```
To this:
```js
var hideSheet = $ionicActionSheet.show({...});
hideSheet();
```
This commit is contained in:
Andrew Joslin
@@ -51,7 +51,6 @@ module.exports = {
|
||||
// Views
|
||||
'js/views/view.js',
|
||||
'js/views/scrollView.js',
|
||||
'js/views/actionSheetView.js',
|
||||
'js/views/headerBarView.js',
|
||||
'js/views/listView.js',
|
||||
'js/views/modalView.js',
|
||||
|
||||
217
js/angular/service/actionSheet.js
vendored
217
js/angular/service/actionSheet.js
vendored
@@ -16,13 +16,13 @@
|
||||
*
|
||||
* ```js
|
||||
* angular.module('mySuperApp', ['ionic'])
|
||||
* .controller(function($scope, $ionicActionSheet) {
|
||||
* .controller(function($scope, $ionicActionSheet, $timeout) {
|
||||
*
|
||||
* // Triggered on a button click, or some other target
|
||||
* $scope.show = function() {
|
||||
*
|
||||
* // Show the action sheet
|
||||
* $ionicActionSheet.show({
|
||||
* var hideSheet = $ionicActionSheet({
|
||||
* buttons: [
|
||||
* { text: '<b>Share</b> This' },
|
||||
* { text: 'Move' },
|
||||
@@ -35,6 +35,11 @@
|
||||
* }
|
||||
* });
|
||||
*
|
||||
* // For example's sake, hide the sheet after two seconds
|
||||
* $timeout(function() {
|
||||
* hideSheet();
|
||||
* }, 2000);
|
||||
*
|
||||
* };
|
||||
* });
|
||||
* ```
|
||||
@@ -52,101 +57,119 @@ IonicModule
|
||||
function($rootScope, $document, $compile, $animate, $timeout, $ionicTemplateLoader, $ionicPlatform) {
|
||||
|
||||
return {
|
||||
/**
|
||||
* @ngdoc method
|
||||
* @name $ionicActionSheet#show
|
||||
* @description
|
||||
* Load and return a new action sheet.
|
||||
*
|
||||
* A new isolated scope will be created for the
|
||||
* action sheet and the new element will be appended into the body.
|
||||
*
|
||||
* @param {object} opts The options for this ActionSheet. Properties:
|
||||
*
|
||||
* - `[Object]` `buttons` Which buttons to show. Each button is an object with a `text` field.
|
||||
* - `{string}` `titleText` The title to show on the action sheet.
|
||||
* - `{string=}` `cancelText` The text for a 'cancel' button on the action sheet.
|
||||
* - `{string=}` `destructiveText` The text for a 'danger' on the action sheet.
|
||||
* - `{function=}` `cancel` Called if the cancel button is pressed, the backdrop is tapped or
|
||||
* the hardware back button is pressed.
|
||||
* - `{function=}` `buttonClicked` Called when one of the non-destructive buttons is clicked,
|
||||
* with the index of the button that was clicked and the button object. Return true to close
|
||||
* the action sheet, or false to keep it opened.
|
||||
* - `{function=}` `destructiveButtonClicked` Called when the destructive button is clicked.
|
||||
* Return true to close the action sheet, or false to keep it opened.
|
||||
*/
|
||||
show: function(opts) {
|
||||
var scope = $rootScope.$new(true);
|
||||
|
||||
angular.extend(scope, opts);
|
||||
|
||||
// Compile the template
|
||||
var element = $compile('<ion-action-sheet buttons="buttons"></ion-action-sheet>')(scope);
|
||||
|
||||
// Grab the sheet element for animation
|
||||
var sheetEl = jqLite(element[0].querySelector('.action-sheet-wrapper'));
|
||||
|
||||
// removes the actionSheet from the screen
|
||||
var hideSheet = function(h) {
|
||||
sheetEl.removeClass('action-sheet-up');
|
||||
|
||||
$animate.removeClass(element, 'active', function() {
|
||||
scope.$destroy();
|
||||
});
|
||||
|
||||
$document[0].body.classList.remove('action-sheet-open');
|
||||
|
||||
scope.$deregisterBackButton && scope.$deregisterBackButton();
|
||||
};
|
||||
|
||||
// registerBackButtonAction returns a callback to deregister the action
|
||||
scope.$deregisterBackButton = $ionicPlatform.registerBackButtonAction(
|
||||
function(){
|
||||
scope.cancel(); //
|
||||
},
|
||||
PLATFORM_BACK_BUTTON_PRIORITY_ACTION_SHEET
|
||||
);
|
||||
|
||||
// called when the user presses the cancel button
|
||||
scope.cancel = function() {
|
||||
hideSheet();
|
||||
$timeout(function(){
|
||||
// after the animation is out, call the cancel callback
|
||||
opts.cancel && opts.cancel();
|
||||
},200)
|
||||
};
|
||||
|
||||
scope.buttonClicked = function(index) {
|
||||
// Check if the button click event returned true, which means
|
||||
// we can close the action sheet
|
||||
if((opts.buttonClicked && opts.buttonClicked(index, opts.buttons[index])) === true) {
|
||||
hideSheet();
|
||||
}
|
||||
};
|
||||
|
||||
scope.destructiveButtonClicked = function() {
|
||||
// Check if the destructive button click event returned true, which means
|
||||
// we can close the action sheet
|
||||
if((opts.destructiveButtonClicked && opts.destructiveButtonClicked()) === true) {
|
||||
hideSheet();
|
||||
}
|
||||
};
|
||||
|
||||
$document[0].body.appendChild(element[0]);
|
||||
|
||||
$document[0].body.classList.add('action-sheet-open');
|
||||
|
||||
var sheet = new ionic.views.ActionSheet({el: element[0] });
|
||||
scope.sheet = sheet;
|
||||
|
||||
$animate.addClass(element, 'active');
|
||||
|
||||
$timeout(function(){
|
||||
sheetEl.addClass('action-sheet-up');
|
||||
}, 20);
|
||||
|
||||
return sheet;
|
||||
}
|
||||
show: actionSheet
|
||||
};
|
||||
|
||||
/**
|
||||
* @ngdoc method
|
||||
* @name $ionicActionSheet#show
|
||||
* @description
|
||||
* Load and return a new action sheet.
|
||||
*
|
||||
* A new isolated scope will be created for the
|
||||
* action sheet and the new element will be appended into the body.
|
||||
*
|
||||
* @param {object} options The options for this ActionSheet. Properties:
|
||||
*
|
||||
* - `[Object]` `buttons` Which buttons to show. Each button is an object with a `text` field.
|
||||
* - `{string}` `titleText` The title to show on the action sheet.
|
||||
* - `{string=}` `cancelText` The text for a 'cancel' button on the action sheet.
|
||||
* - `{string=}` `destructiveText` The text for a 'danger' on the action sheet.
|
||||
* - `{function=}` `cancel` Called if the cancel button is pressed, the backdrop is tapped or
|
||||
* the hardware back button is pressed.
|
||||
* - `{function=}` `buttonClicked` Called when one of the non-destructive buttons is clicked,
|
||||
* with the index of the button that was clicked and the button object. Return true to close
|
||||
* the action sheet, or false to keep it opened.
|
||||
* - `{function=}` `destructiveButtonClicked` Called when the destructive button is clicked.
|
||||
* Return true to close the action sheet, or false to keep it opened.
|
||||
*
|
||||
* @returns {function} `hideSheet` A function which, when called, hides & cancels the action sheet.
|
||||
*/
|
||||
function actionSheet(opts) {
|
||||
var scope = $rootScope.$new(true);
|
||||
|
||||
angular.extend(scope, {
|
||||
cancel: angular.noop,
|
||||
destructiveButtonClicked: angular.noop,
|
||||
buttonClicked: angular.noop,
|
||||
$deregisterBackButton: angular.noop,
|
||||
buttons: [],
|
||||
}, opts || {});
|
||||
|
||||
// Compile the template
|
||||
var element = scope.element = $compile('<ion-action-sheet buttons="buttons"></ion-action-sheet>')(scope);
|
||||
|
||||
// Grab the sheet element for animation
|
||||
var sheetEl = jqLite(element[0].querySelector('.action-sheet-wrapper'));
|
||||
|
||||
// removes the actionSheet from the screen
|
||||
scope.removeSheet = function(done) {
|
||||
if (scope.removed) return;
|
||||
|
||||
scope.removed = true;
|
||||
sheetEl.removeClass('action-sheet-up');
|
||||
$document[0].body.classList.remove('action-sheet-open');
|
||||
scope.$deregisterBackButton();
|
||||
|
||||
$animate.removeClass(element, 'active', function() {
|
||||
scope.$destroy();
|
||||
element.remove();
|
||||
// scope.cancel.$scope is defined near the bottom
|
||||
scope.cancel.$scope = null;
|
||||
(done || angular.noop)();
|
||||
});
|
||||
};
|
||||
|
||||
scope.showSheet = function(done) {
|
||||
if (scope.removed) return;
|
||||
|
||||
$document[0].body.appendChild(element[0]);
|
||||
$document[0].body.classList.add('action-sheet-open');
|
||||
|
||||
$animate.addClass(element, 'active', function() {
|
||||
if (scope.removed) return;
|
||||
(done || angular.noop)();
|
||||
});
|
||||
$timeout(function(){
|
||||
if (scope.removed) return;
|
||||
sheetEl.addClass('action-sheet-up');
|
||||
}, 20, false);
|
||||
};
|
||||
|
||||
// registerBackButtonAction returns a callback to deregister the action
|
||||
scope.$deregisterBackButton = $ionicPlatform.registerBackButtonAction(
|
||||
scope.cancel,
|
||||
PLATFORM_BACK_BUTTON_PRIORITY_ACTION_SHEET
|
||||
);
|
||||
|
||||
// called when the user presses the cancel button
|
||||
scope.cancel = function() {
|
||||
// after the animation is out, call the cancel callback
|
||||
scope.removeSheet(opts.cancel);
|
||||
};
|
||||
|
||||
scope.buttonClicked = function(index) {
|
||||
// Check if the button click event returned true, which means
|
||||
// we can close the action sheet
|
||||
if (opts.buttonClicked(index, opts.buttons[index]) === true) {
|
||||
scope.removeSheet();
|
||||
}
|
||||
};
|
||||
|
||||
scope.destructiveButtonClicked = function() {
|
||||
// Check if the destructive button click event returned true, which means
|
||||
// we can close the action sheet
|
||||
if (opts.destructiveButtonClicked() === true) {
|
||||
scope.removeSheet();
|
||||
}
|
||||
};
|
||||
|
||||
scope.showSheet();
|
||||
|
||||
// Expose the scope on $ionicActionSheet's return value for the sake
|
||||
// of testing it.
|
||||
scope.cancel.$scope = scope;
|
||||
|
||||
return scope.cancel;
|
||||
}
|
||||
}]);
|
||||
|
||||
@@ -1,26 +0,0 @@
|
||||
(function(ionic) {
|
||||
'use strict';
|
||||
/**
|
||||
* An ActionSheet is the slide up menu popularized on iOS.
|
||||
*
|
||||
* You see it all over iOS apps, where it offers a set of options
|
||||
* triggered after an action.
|
||||
*/
|
||||
ionic.views.ActionSheet = ionic.views.View.inherit({
|
||||
initialize: function(opts) {
|
||||
this.el = opts.el;
|
||||
},
|
||||
show: function() {
|
||||
// Force a reflow so the animation will actually run
|
||||
this.el.offsetWidth;
|
||||
|
||||
this.el.classList.add('active');
|
||||
},
|
||||
hide: function() {
|
||||
// Force a reflow so the animation will actually run
|
||||
this.el.offsetWidth;
|
||||
this.el.classList.remove('active');
|
||||
}
|
||||
});
|
||||
|
||||
})(ionic);
|
||||
@@ -1,44 +1,81 @@
|
||||
describe('Ionic ActionSheet Service', function() {
|
||||
|
||||
var sheet, timeout, ionicPlatform;
|
||||
|
||||
beforeEach(module('ionic'));
|
||||
|
||||
beforeEach(inject(function($ionicActionSheet, $timeout, $ionicPlatform) {
|
||||
sheet = $ionicActionSheet;
|
||||
timeout = $timeout;
|
||||
ionicPlatform = $ionicPlatform;
|
||||
beforeEach(module('ionic', function($provide) {
|
||||
// For the sake of this test, we don't want ionActionSheet to
|
||||
// actually compile as a directive.
|
||||
// We are only testing the service.
|
||||
$provide.value('ionActionSheetDirective', []);
|
||||
}));
|
||||
|
||||
it('Should show', function() {
|
||||
var s = sheet.show();
|
||||
expect(s.el.classList.contains('active')).toBe(true);
|
||||
});
|
||||
function setup(options) {
|
||||
var scope;
|
||||
inject(function($ionicActionSheet, $ionicPlatform, $timeout) {
|
||||
var hide = $ionicActionSheet.show(options || {});
|
||||
$timeout.flush();
|
||||
scope = hide.$scope;
|
||||
});
|
||||
return scope;
|
||||
}
|
||||
|
||||
it('Should add .action-sheet-up to .action-sheet-wrapper', function() {
|
||||
var s = sheet.show();
|
||||
var el = angular.element(s.el);
|
||||
var wrapper = angular.element(s.el.querySelector('.action-sheet-wrapper'));
|
||||
expect(wrapper.length).toEqual(1);
|
||||
expect(wrapper.hasClass('action-sheet-up')).toEqual(false);
|
||||
timeout.flush();
|
||||
expect(wrapper.hasClass('action-sheet-up')).toEqual(true);
|
||||
});
|
||||
|
||||
it('should handle hardware back button', function() {
|
||||
var s = sheet.show();
|
||||
|
||||
ionicPlatform.hardwareBackButtonClick();
|
||||
|
||||
expect(s.el.classList.contains('active')).toBe(false);
|
||||
});
|
||||
|
||||
it('show & hide should add action-sheet-open to body', inject(function($animate) {
|
||||
var s = sheet.show();
|
||||
|
||||
expect(angular.element(document.body).hasClass('action-sheet-open')).toBe(true);
|
||||
|
||||
ionicPlatform.hardwareBackButtonClick();
|
||||
|
||||
expect(angular.element(document.body).hasClass('action-sheet-open')).toBe(false);
|
||||
it('should add classes on showing', inject(function($document) {
|
||||
var scope = setup();
|
||||
expect($document[0].body.classList.contains('action-sheet-open')).toBe(true);
|
||||
expect(scope.element.hasClass('active')).toBe(true);
|
||||
}));
|
||||
|
||||
it('removeSheet should remove classes, remove element and destroy scope', inject(function($document, $timeout) {
|
||||
var scope = setup();
|
||||
spyOn(scope, '$destroy');
|
||||
spyOn(scope.element, 'remove');
|
||||
scope.removeSheet();
|
||||
expect($document[0].body.classList.contains('action-sheet-open')).toBe(false);
|
||||
expect(scope.element.hasClass('active')).toBe(false);
|
||||
$timeout.flush();
|
||||
expect(scope.$destroy).toHaveBeenCalled();
|
||||
expect(scope.element.remove).toHaveBeenCalled();
|
||||
}));
|
||||
|
||||
it('destructiveButtonClicked should removeSheet if returning true', function() {
|
||||
var destructiveReturnValue = false;
|
||||
var scope = setup({
|
||||
destructiveButtonClicked: function() {
|
||||
return destructiveReturnValue;
|
||||
}
|
||||
});
|
||||
spyOn(scope, 'removeSheet');
|
||||
scope.destructiveButtonClicked();
|
||||
expect(scope.removeSheet).not.toHaveBeenCalled();
|
||||
destructiveReturnValue = true;
|
||||
scope.destructiveButtonClicked();
|
||||
expect(scope.removeSheet).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('buttonClicked should removeSheet if returning true for index', function() {
|
||||
var scope = setup({
|
||||
buttons: [{}, {}],
|
||||
buttonClicked: function(index) {
|
||||
return index === 0 ? false : true;
|
||||
}
|
||||
});
|
||||
spyOn(scope, 'removeSheet');
|
||||
scope.buttonClicked(0);
|
||||
expect(scope.removeSheet).not.toHaveBeenCalled();
|
||||
scope.buttonClicked(1);
|
||||
expect(scope.removeSheet).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('cancel should removeSheet and call opts.cancel', inject(function($timeout) {
|
||||
var cancelSpy = jasmine.createSpy('opts.cancel');
|
||||
var scope = setup({
|
||||
cancel: cancelSpy
|
||||
});
|
||||
spyOn(scope, 'removeSheet').andCallThrough();
|
||||
scope.cancel();
|
||||
expect(scope.removeSheet).toHaveBeenCalled();
|
||||
$timeout.flush();
|
||||
expect(cancelSpy).toHaveBeenCalled();
|
||||
}));
|
||||
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user