Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Refactor carousel to work with ngAnimate #2699

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 63 additions & 3 deletions Gruntfile.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* jshint node: true */
var markdown = require('node-markdown').Markdown;
var fs = require('fs');

module.exports = function(grunt) {

Expand Down Expand Up @@ -29,6 +30,9 @@ module.exports = function(grunt) {
modules: 'angular.module("ui.bootstrap", [<%= srcModules %>]);',
tplmodules: 'angular.module("ui.bootstrap.tpls", [<%= tplModules %>]);',
all: 'angular.module("ui.bootstrap", ["ui.bootstrap.tpls", <%= srcModules %>]);',
cssInclude: '',
cssFileBanner: '/* Include this file in your html if you are using the CSP mode. */\n\n',
cssFileDest: '<%= dist %>/<%= filename %>-<%= pkg.version %>-csp.css',
banner: ['/*',
' * <%= pkg.name %>',
' * <%= pkg.homepage %>\n',
Expand All @@ -54,14 +58,16 @@ module.exports = function(grunt) {
concat: {
dist: {
options: {
banner: '<%= meta.banner %><%= meta.modules %>\n'
banner: '<%= meta.banner %><%= meta.modules %>\n',
footer: '<%= meta.cssInclude %>'
},
src: [], //src filled in by build task
dest: '<%= dist %>/<%= filename %>-<%= pkg.version %>.js'
},
dist_tpls: {
options: {
banner: '<%= meta.banner %><%= meta.all %>\n<%= meta.tplmodules %>\n'
banner: '<%= meta.banner %><%= meta.all %>\n<%= meta.tplmodules %>\n',
footer: '<%= meta.cssInclude %>'
},
src: [], //src filled in by build task
dest: '<%= dist %>/<%= filename %>-tpls-<%= pkg.version %>.js'
Expand Down Expand Up @@ -246,6 +252,7 @@ module.exports = function(grunt) {
moduleName: enquote('ui.bootstrap.' + name),
displayName: ucwords(breakup(name, ' ')),
srcFiles: grunt.file.expand('src/'+name+'/*.js'),
cssFiles: grunt.file.expand('src/'+name+'/*.css'),
tplFiles: grunt.file.expand('template/'+name+'/*.html'),
tpljsFiles: grunt.file.expand('template/'+name+'/*.html.js'),
tplModules: grunt.file.expand('template/'+name+'/*.html').map(enquote),
Expand All @@ -259,6 +266,17 @@ module.exports = function(grunt) {
.map(grunt.file.read).join('\n')
}
};

var styles = {
css: [],
js: []
};
module.cssFiles.forEach(processCSS.bind(null, styles, true));
if (styles.css.length) {
module.css = styles.css.join('\n');
module.cssJs = styles.js.join('\n');
}

module.dependencies.forEach(findModule);
grunt.config('modules', grunt.config('modules').concat(module));
}
Expand Down Expand Up @@ -322,6 +340,17 @@ module.exports = function(grunt) {
})
);

var cssStrings = _.flatten(_.compact(_.pluck(modules, 'css')));
var cssJsStrings = _.flatten(_.compact(_.pluck(modules, 'cssJs')));
if (cssStrings.length) {
grunt.config('meta.cssInclude', cssJsStrings.join('\n'));

grunt.file.write(grunt.config('meta.cssFileDest'), grunt.config('meta.cssFileBanner') +
cssStrings.join('\n'));

grunt.log.writeln('File ' + grunt.config('meta.cssFileDest') + ' created');
}

var moduleFileMapping = _.clone(modules, true);
moduleFileMapping.forEach(function (module) {
delete module.docs;
Expand Down Expand Up @@ -374,9 +403,40 @@ module.exports = function(grunt) {
var genRawFilesJs = require('./misc/raw-files-generator');

genRawFilesJs(grunt, jsFilename, _.flatten(grunt.config('concat.dist_tpls.src')),
grunt.config('meta.banner'));
grunt.config('meta.banner'), grunt.config('meta.cssFileBanner'));
});

/**
* Logic from AngularJS
* https://github.com/angular/angular.js/blob/36831eccd1da37c089f2141a2c073a6db69f3e1d/lib/grunt/utils.js#L121-L145
*/
function processCSS(state, minify, file) {
/* jshint quotmark: false */
var css = fs.readFileSync(file).toString(),
js;
state.css.push(css);

if(minify){
css = css
.replace(/\r?\n/g, '')
.replace(/\/\*.*?\*\//g, '')
.replace(/:\s+/g, ':')
.replace(/\s*\{\s*/g, '{')
.replace(/\s*\}\s*/g, '}')
.replace(/\s*\,\s*/g, ',')
.replace(/\s*\;\s*/g, ';');
}
//escape for js
css = css
.replace(/\\/g, '\\\\')
.replace(/'/g, "\\'")
.replace(/\r?\n/g, '\\n');
js = "!angular.$$csp() && angular.element(document).find('head').prepend('<style type=\"text/css\">" + css + "</style>');";
state.js.push(js);

return state;
}

function setVersion(type, suffix) {
var file = 'package.json';
var VERSION_REGEX = /([\'|\"]version[\'|\"][ ]*:[ ]*[\'|\"])([\d|.]*)(-\w+)*([\'|\"])/;
Expand Down
37 changes: 32 additions & 5 deletions misc/demo/assets/app.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* global FastClick, smoothScroll */
angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch'], function($httpProvider){
angular.module('ui.bootstrap.demo', ['ui.bootstrap', 'plunker', 'ngTouch', 'ngAnimate'], function($httpProvider){
FastClick.attach(document.body);
delete $httpProvider.defaults.headers.common['X-Requested-With'];
}).run(['$location', function($location){
Expand Down Expand Up @@ -156,11 +156,38 @@ var SelectModulesCtrl = function($scope, $modalInstance, modules, buildFilesServ

var jsTplFile = createWithTplFile(srcModuleFullNames, srcJsContent, tplModuleNames, tplJsContent);

var cssContent = srcModules
.map(function (module) {
return module.css;
})
.filter(function (css) {
return css;
})
.join('\n')
;

var cssJsContent = srcModules
.map(function (module) {
return module.cssJs;
})
.filter(function (cssJs) {
return cssJs;
})
.join('\n')
;

var footer = cssJsContent;

var zip = new JSZip();
zip.file('ui-bootstrap-custom-' + version + '.js', rawFiles.banner + jsFile);
zip.file('ui-bootstrap-custom-' + version + '.min.js', rawFiles.banner + uglify(jsFile));
zip.file('ui-bootstrap-custom-tpls-' + version + '.js', rawFiles.banner + jsTplFile);
zip.file('ui-bootstrap-custom-tpls-' + version + '.min.js', rawFiles.banner + uglify(jsTplFile));
zip.file('ui-bootstrap-custom-' + version + '.js', rawFiles.banner + jsFile + footer);
zip.file('ui-bootstrap-custom-' + version + '.min.js', rawFiles.banner + uglify(jsFile + footer));
zip.file('ui-bootstrap-custom-tpls-' + version + '.js', rawFiles.banner + jsTplFile + footer);
zip.file('ui-bootstrap-custom-tpls-' + version + '.min.js', rawFiles.banner + uglify(jsTplFile + footer));
zip.file('ui-bootstrap-custom-tpls-' + version + '.min.js', rawFiles.banner + uglify(jsTplFile + footer));

if (cssContent) {
zip.file('ui-bootstrap-custom-' + version + '-csp.css', rawFiles.cssBanner + cssContent);
}

saveAs(zip.generate({type: 'blob'}), 'ui-bootstrap-custom-build.zip');
}
Expand Down
1 change: 1 addition & 0 deletions misc/demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<script src="//cdnjs.cloudflare.com/ajax/libs/FileSaver.js/1.0.0/FileSaver.min.js"></script>
<script src="//cdnjs.cloudflare.com/ajax/libs/jszip/2.4.0/jszip.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-animate.min.js"></script>
<script src="//ajax.googleapis.com/ajax/libs/angularjs/<%= ngversion %>/angular-touch.min.js"></script>
<script src="ui-bootstrap-tpls-<%= pkg.version%>.min.js"></script>
<script src="assets/plunker.js"></script>
Expand Down
7 changes: 6 additions & 1 deletion misc/raw-files-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ function getFiles(filePaths) {
return files;
}

module.exports = function generateRawFilesJs(grunt, jsFilename, files, banner) {
module.exports = function generateRawFilesJs(grunt, jsFilename, files, banner, cssBanner) {
if (!banner) {
banner = '';
}

if (!cssBanner) {
cssBanner = '';
}

var filesJsObject = {
banner: banner,
cssBanner: cssBanner,
files: getFiles(files),
};

Expand Down
4 changes: 4 additions & 0 deletions src/carousel/carousel.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.ng-animate.item:not(.left):not(.right) {
-webkit-transition: 0s ease-in-out left;
transition: 0s ease-in-out left
}
100 changes: 65 additions & 35 deletions src/carousel/carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*
*/
angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
.controller('CarouselController', ['$scope', '$timeout', '$interval', '$transition', function ($scope, $timeout, $interval, $transition) {
.controller('CarouselController', ['$scope', '$interval', '$animate', function ($scope, $interval, $animate) {
var self = this,
slides = self.slides = $scope.slides = [],
currentIndex = -1,
Expand All @@ -23,51 +23,30 @@ angular.module('ui.bootstrap.carousel', ['ui.bootstrap.transition'])
direction = nextIndex > currentIndex ? 'next' : 'prev';
}
if (nextSlide && nextSlide !== self.currentSlide) {
if ($scope.$currentTransition) {
$scope.$currentTransition.cancel();
//Timeout so ng-class in template has time to fix classes for finished slide
$timeout(goNext);
} else {
goNext();
}
goNext();
}
function goNext() {
// Scope has been destroyed, stop here.
if (destroyed) { return; }
//If we have a slide to transition from and we have a transition type and we're allowed, go
if (self.currentSlide && angular.isString(direction) && !$scope.noTransition && nextSlide.$element) {
//We shouldn't do class manip in here, but it's the same weird thing bootstrap does. need to fix sometime
nextSlide.$element.addClass(direction);
var reflow = nextSlide.$element[0].offsetWidth; //force reflow

//Set all other slides to stop doing their stuff for the new transition
angular.forEach(slides, function(slide) {
angular.extend(slide, {direction: '', entering: false, leaving: false, active: false});
});
angular.extend(nextSlide, {direction: direction, active: true, entering: true});
angular.extend(self.currentSlide||{}, {direction: direction, leaving: true});
angular.extend(nextSlide, {direction: direction, active: true});
angular.extend(self.currentSlide || {}, {direction: direction, active: false});

$scope.$currentTransition = $transition(nextSlide.$element, {});
//We have to create new pointers inside a closure since next & current will change
(function(next,current) {
$scope.$currentTransition.then(
function(){ transitionDone(next, current); },
function(){ transitionDone(next, current); }
);
}(nextSlide, self.currentSlide));
} else {
transitionDone(nextSlide, self.currentSlide);
if ($animate.enabled() && !$scope.noTransition && nextSlide.$element) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only question I have about this check is that there is the potential for nextSlide.$element.data('$$ngAnimateState').disabled to be true - in that case, this would be broken. Do we want to say that we do not support this use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think if I do this on the $animate:before event for any slide element (and attach it in the slide directive)?
Since it should only mark $currentTransition = true when the animation has started, and it'll only close if it starts. There's a possible race condition where an animation hasn't started but has been queued, but may not matter so much I think, as it's only meant to prevent double-clicking causing a weird transition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I was just looking at the animation code (in https://github.com/angular/angular.js/blob/g3_v1_2/src/ngAnimate/animate.js#L856-L861), I think the close event always even if the animation is disabled for the element.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're right - ignore my comment.

$scope.$currentTransition = true;
// TODO: Switch to use .one when upgrading beyond 1.2.21
// (See https://github.com/angular/angular.js/pull/5984)
nextSlide.$element.on('$animate:close', function closeFn() {
$scope.$currentTransition = null;
nextSlide.$element.off('$animate:close', closeFn);
});
}

self.currentSlide = nextSlide;
currentIndex = nextIndex;
//every time you change slides, reset the timer
restartTimer();
}
function transitionDone(next, current) {
angular.extend(next, {direction: '', active: true, leaving: false, entering: false});
angular.extend(current||{}, {direction: '', active: false, leaving: false, entering: false});
$scope.$currentTransition = null;
}
};
$scope.$on('$destroy', function () {
destroyed = true;
Expand Down Expand Up @@ -290,4 +269,55 @@ function CarouselDemoCtrl($scope) {
});
}
};
});
})

.animation('.item', [
'$animate',
function ($animate) {
return {
beforeAddClass: function (element, className, done) {
// Due to transclusion, noTransition property is on parent's scope
if (className == 'active' && element.parent() &&
!element.parent().scope().noTransition) {
var stopped = false;
var direction = element.isolateScope().direction;
var directionClass = direction == 'next' ? 'left' : 'right';
element.addClass(direction);
$animate.addClass(element, directionClass).then(function () {
if (!stopped) {
element.removeClass(directionClass + ' ' + direction);
}
done();
});

return function () {
stopped = true;
};
}
done();
},
beforeRemoveClass: function (element, className, done) {
// Due to transclusion, noTransition property is on parent's scope
if (className == 'active' && element.parent() &&
!element.parent().scope().noTransition) {
var stopped = false;
var direction = element.isolateScope().direction;
var directionClass = direction == 'next' ? 'left' : 'right';
$animate.addClass(element, directionClass).then(function () {
if (!stopped) {
element.removeClass(directionClass);
}
done();
});
return function () {
stopped = true;
};
}
done();
}
};

}])


;
6 changes: 1 addition & 5 deletions template/carousel/slide.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
<div ng-class="{
'active': leaving || (active && !entering),
'prev': (next || active) && direction=='prev',
'next': (next || active) && direction=='next',
'right': direction=='prev',
'left': direction=='next'
'active': active
}" class="item text-center" ng-transclude></div>