Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(mdToolbar): Allow md-scroll-shrink usage with ng-if.
Browse files Browse the repository at this point in the history 
Previously, the toolbar would fail to enable scroll shrinking
if the developer used an `ng-if` statement on the `md-toolbar`.

This commit allows usage of `ng-if` as well as watching the
`md-scroll-shrink` for changes so they can enable/disable
scroll shrinking.

Fixes #2751. Closes #4394.
  • Loading branch information
@topherfangio @ThomasBurleson
topherfangio authored and ThomasBurleson committed Sep 1, 2015
1 parent 44a7e52 commit 9b861fd
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 23 deletions.
86 changes: 68 additions & 18 deletions src/components/toolbar/toolbar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,22 @@ angular.module('material.components.toolbar', [
* at one fourth the rate at which the user scrolls down. Default 0.5.
*/

function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate ) {
var translateY = angular.bind(null, $mdUtil.supplant, 'translate3d(0,{0}px,0)' );
function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate) {
var translateY = angular.bind(null, $mdUtil.supplant, 'translate3d(0,{0}px,0)');

return {
restrict: 'E',
template: '<div ng-transclude></div>',
transclude: true,

This comment has been minimized.

Sign in to view

Copy link
@daniel-nagy

daniel-nagy Sep 1, 2015

Contributor

This breaks toolbars that use ng-include or ng-controller. Of course the developer could wrap there toolbar in an extra div and place their ng-include or ng-controller attributes on that element but then you end up with two extra unnecessary divs

<md-toolbar>
  <div ng-transclude>
    <div ng-include="my-toolbar-template.html">
      <!-- actual toolbar content -->
    </div>
  </div>
</md-toolbar>

I think most developers would expect to be able to put ng-include or ng-controller attributes directly on the md-toolbar element.

Maybe instead of using transclusion you can manually wrap the contents of the md-toolbar element in an extra div if necessary. In addition, the isolated scope is going to break putting ng-controller directly on the md-toolbar element as well. Maybe you can use bindToController instead and create a controller to handle scroll shrink?
controller: angular.noop,
scope: {
scrollShrink: '=?mdScrollShrink'
},
link: function(scope, element, attr) {

$mdTheming(element);

if (angular.isDefined(attr.mdScrollShrink)) {
setupScrollShrink();
}
setupScrollShrink();

function setupScrollShrink() {
// Current "y" position of scroll
Expand All @@ -84,24 +87,68 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate )
var debouncedContentScroll = $$rAF.throttle(onContentScroll);
var debouncedUpdateHeight = $mdUtil.debounce(updateToolbarHeight, 5 * 1000);

var disableScrollShrink;

// Wait for $mdContentLoaded event from mdContent directive.
// If the mdContent element is a sibling of our toolbar, hook it up
// to scroll events.
scope.$on('$mdContentLoaded', onMdContentLoad);

// If the toolbar is used inside an ng-if statement, we may miss the
// $mdContentLoaded event, so we attempt to fake it if we have a
// md-content close enough.
scope.$watch('scrollShrink', onChangeScrollShrink);

// If the scope is destroyed (which could happen with ng-if), make sure
// to disable scroll shrinking again
scope.$on('$destroy', function() {
disableScrollShrink();
});

function onChangeScrollShrink(scrollShrink) {
var closestContent = element.parent().find('md-content');

// If we have a content element, fake the call; this might still fail
// if the content element isn't a sibling of the toolbar
if (!contentElement && closestContent.length) {
onMdContentLoad(null, closestContent);
}

if (contentElement) {
// Disable only if the attribute's expression evaluates to false
if (scrollShrink === false) {
disableScrollShrink();
} else {
enableScrollShrink();
}
}
}

function enableScrollShrink() {
contentElement.on('scroll', debouncedContentScroll);
contentElement.attr('scroll-shrink', 'true');

$$rAF(updateToolbarHeight);

return function disableScrollShrink() {
contentElement.off('scroll', debouncedContentScroll);
contentElement.attr('scroll-shrink', 'false');

$$rAF(updateToolbarHeight);
}
}


function onMdContentLoad($event, newContentEl) {
// Toolbar and content must be siblings
if (element.parent()[0] === newContentEl.parent()[0]) {
if (newContentEl && element.parent()[0] === newContentEl.parent()[0]) {
// unhook old content event listener if exists
if (contentElement) {
contentElement.off('scroll', debouncedContentScroll);
}

newContentEl.on('scroll', debouncedContentScroll);
newContentEl.attr('scroll-shrink', 'true');

contentElement = newContentEl;
$$rAF(updateToolbarHeight);
disableScrollShrink = enableScrollShrink();
}
}

Expand All @@ -113,9 +160,12 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate )
//
// As the user scrolls down, the content will be transformed up slowly
// to put the content underneath where the toolbar was.
var margin = (-toolbarHeight * shrinkSpeedFactor) + 'px';
contentElement.css('margin-top', margin);
contentElement.css('margin-bottom', margin);
var margin = (-toolbarHeight * shrinkSpeedFactor) + 'px';

contentElement.css({
"margin-top": margin,
"margin-bottom": margin
});

onContentScroll();
}
Expand All @@ -130,17 +180,17 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate )
Math.max(0, y + scrollTop - prevScrollTop)
);

element.css( $mdConstant.CSS.TRANSFORM, translateY([-y * shrinkSpeedFactor]) );
contentElement.css( $mdConstant.CSS.TRANSFORM, translateY([(toolbarHeight - y) * shrinkSpeedFactor]) );
element.css($mdConstant.CSS.TRANSFORM, translateY([-y * shrinkSpeedFactor]));
contentElement.css($mdConstant.CSS.TRANSFORM, translateY([(toolbarHeight - y) * shrinkSpeedFactor]));

prevScrollTop = scrollTop;

$mdUtil.nextTick(function () {
$mdUtil.nextTick(function() {
var hasWhiteFrame = element.hasClass('md-whiteframe-z1');

if ( hasWhiteFrame && !y) {
if (hasWhiteFrame && !y) {
$animate.removeClass(element, 'md-whiteframe-z1');
} else if ( !hasWhiteFrame && y ) {
} else if (!hasWhiteFrame && y) {
$animate.addClass(element, 'md-whiteframe-z1');
}
});
Expand Down
99 changes: 94 additions & 5 deletions src/components/toolbar/toolbar.spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
describe('<md-toolbar>', function() {

var pageScope, element, controller;
var $rootScope, $timeout;

beforeEach(module('material.components.toolbar'));
beforeEach(inject(function(_$rootScope_, _$timeout_) {
$rootScope = _$rootScope_;
$timeout = _$timeout_;
}));

it('with scrollShrink, it should shrink scrollbar when going to bottom', inject(function($compile, $rootScope, $mdConstant, mdToolbarDirective, $$rAF) {

Expand All @@ -23,12 +30,20 @@ describe('<md-toolbar>', function() {
toolbarCss[k] = v;
});
var contentCss = {};
spyOn(contentEl, 'css').and.callFake(function(k, v) {
contentCss[k] = v;
spyOn(contentEl, 'css').and.callFake(function(properties, value) {
if (angular.isObject(properties)) {
for (k in properties) {
if (properties.hasOwnProperty(k)) {
contentCss[k] = properties[k];
}
}
} else {
contentCss[properties] = value;
}
});

// Manually link so we can give our own elements with spies on them
mdToolbarDirective[0].link($rootScope, toolbar, {
mdToolbarDirective[0].link($rootScope, toolbar, {
mdScrollShrink: true,
mdShrinkSpeedFactor: 1
});
Expand All @@ -47,7 +62,7 @@ describe('<md-toolbar>', function() {
// Fake scroll to the bottom
contentEl.triggerHandler({
type: 'scroll',
target: { scrollTop: 500 }
target: {scrollTop: 500}
});
$$rAF.flush();

Expand All @@ -57,12 +72,86 @@ describe('<md-toolbar>', function() {
// Fake scroll back to the top
contentEl.triggerHandler({
type: 'scroll',
target: { scrollTop: 0 }
target: {scrollTop: 0}
});
$$rAF.flush();

expect(toolbarCss[$mdConstant.CSS.TRANSFORM]).toEqual('translate3d(0,0px,0)');
expect(contentCss[$mdConstant.CSS.TRANSFORM]).toEqual('translate3d(0,100px,0)');

}));

it('works without ng-if', inject(function() {
build(
'<div>' +
' <md-toolbar md-scroll-shrink="true"></md-toolbar>' +
' <md-content></md-content>' +
'</div>'
);

expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');
}));

it('works with ng-if', inject(function() {
build(
'<div>' +
' <md-toolbar md-scroll-shrink="true" ng-if="shouldShowToolbar"></md-toolbar>' +
' <md-content></md-content>' +
'</div>'
);

// It starts out undefined
expect(element.find('md-content').attr('scroll-shrink')).toEqual(undefined);

// Change the ng-if to add the toolbar
pageScope.$apply('shouldShowToolbar = true');
expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');

// Change the ng-if to remove the toolbar
pageScope.$apply('shouldShowToolbar = false');
expect(element.find('md-content').attr('scroll-shrink')).toEqual('false');
}));

it('enables scroll shrink when the attribute has no value', function() {
build(
'<div>' +
' <md-toolbar md-scroll-shrink></md-toolbar>' +
' <md-content></md-content>' +
'</div>'
);

expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');
});

it('watches the value of scroll shrink', function() {
build(
'<div>' +
' <md-toolbar md-scroll-shrink="shouldShrink"></md-toolbar>' +
' <md-content></md-content>' +
'</div>'
);

// It starts out undefined which SHOULD add the scroll shrink because it acts as if no value
// was specified
expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');

// Change the scrollShink to false
pageScope.$apply('shouldShrink = false');
expect(element.find('md-content').attr('scroll-shrink')).toEqual('false');

// Change the scrollShink to true
pageScope.$apply('shouldShrink = true');
expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');
});

function build(template) {
inject(function($compile) {
pageScope = $rootScope.$new();
element = $compile(template)(pageScope);
controller = element.controller('mdToolbar');

pageScope.$apply();
$timeout.flush();
});
}
});

0 comments on commit 9b861fd

Please sign in to comment.