Skip to content

Commit

Permalink
fix(mdSubheader): Non top-level md-content causes incorrect position.
Browse files Browse the repository at this point in the history
If the subheader's parent `<md-content>` was not the top-level item,
the stickied subheader would be incorrectly positioned further down
the page.

Also added more tests to toolbar and fixed issue to ensure subheader
works with a toolbar using `md-scroll-shrink`.

Fixes angular#4420. References angular#2825. Closes angular#4439.
  • Loading branch information
topherfangio authored and kennethcachia committed Sep 23, 2015
1 parent 6f19a04 commit af10598
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 10 deletions.
2 changes: 0 additions & 2 deletions src/components/sticky/sticky.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ function MdSticky($document, $mdConstant, $$rAF, $mdUtil) {
contentEl.on('$scroll', onScroll);

var self;
var stickyBaseoffset = contentEl.prop('offsetTop');
return self = {
prev: null,
current: null, //the currently stickied item
Expand All @@ -86,7 +85,6 @@ function MdSticky($document, $mdConstant, $$rAF, $mdUtil) {
// Add an element and its sticky clone to this content's sticky collection
function add(element, stickyClone) {
stickyClone.addClass('md-sticky-clone');
stickyClone.css('top', stickyBaseoffset + 'px');

var item = {
element: element,
Expand Down
6 changes: 5 additions & 1 deletion src/components/subheader/demoBasicUsage/index.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@

<div ng-controller="SubheaderAppCtrl" layout="column" flex layout-fill>

<md-toolbar md-scroll-shrink>
<div class="md-toolbar-tools">My Messages</div>
</md-toolbar>

<md-content style="height: 600px;" md-theme="altTheme">

<section>
Expand Down
18 changes: 13 additions & 5 deletions src/components/toolbar/toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate) {

$mdTheming(element);

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

function setupScrollShrink() {

Expand Down Expand Up @@ -100,7 +102,7 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate) {
// If the scope is destroyed (which could happen with ng-if), make sure
// to disable scroll shrinking again

scope.$on('$destroy', disableScrollShrink );
scope.$on('$destroy', disableScrollShrink);

/**
*
Expand All @@ -115,9 +117,15 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate) {
onMdContentLoad(null, closestContent);
}

// Disable only if the attribute's expression evaluates to false
// Evaluate the expression
shrinkWithScroll = scope.$eval(shrinkWithScroll);

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

/**
Expand Down Expand Up @@ -170,7 +178,7 @@ function mdToolbarDirective($$rAF, $mdConstant, $mdUtil, $mdTheming, $animate) {
*
*/
function enableScrollShrink() {
if ( !contentElement ) return angular.noop; // no md-content
if (!contentElement) return angular.noop; // no md-content

contentElement.on('scroll', debouncedContentScroll);
contentElement.attr('scroll-shrink', 'true');
Expand Down
43 changes: 41 additions & 2 deletions src/components/toolbar/toolbar.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ describe('<md-toolbar>', function() {
expect($exceptionHandler.errors).toEqual([]);
}));

it('disables scroll shrink when the attribute is not provided', inject(function() {
build(
'<div>' +
' <md-toolbar></md-toolbar>' +
' <md-content></md-content>' +
'</div>'
);

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

it('enables scroll shrink when the attribute has no value', function() {
build(
'<div>' +
Expand All @@ -142,9 +153,37 @@ describe('<md-toolbar>', function() {
expect(element.find('md-content').attr('scroll-shrink')).toEqual('true');
});

function build(template) {
it('disables scroll shrink if the expression evaluates to false', function() {
var pageScope = $rootScope.$new();

// Set the value to false
pageScope.$apply('someValue = false');

// Build the element
build(
// Pass our template
'<div>' +
' <md-toolbar md-scroll-shrink="someValue"></md-toolbar>' +
' <md-content></md-content>' +
'</div>',

// Pass our custom pageScope
pageScope
);

// Check that scroll shrink is disabled
expect(element.find('md-content').attr('scroll-shrink')).toEqual('false');
});


function build(template, scope) {
inject(function($compile) {
pageScope = $rootScope.$new();
if (scope) {
pageScope = scope
} else {
pageScope = $rootScope.$new();
}

element = $compile(template)(pageScope);
controller = element.controller('mdToolbar');

Expand Down

0 comments on commit af10598

Please sign in to comment.