Skip to content

Commit

Permalink
fix(masonry.restrict): change directive restriction to attribute-only
Browse files Browse the repository at this point in the history 
The masonry directive does not properly create columns when used as an element.  To avoid confusion,
I updated the directive code to restrict usage to attribute mode only (this could always be changed
back, if a subsequent fix was made to fix element loading). I have also updated the repository
README documentation to provide a more accurate account of how the masonry directive currently
works. This issue was first brought up here: passy#169

This will invalidate uses of this directive as an element. However, these uses are likely already
  • Loading branch information
@blakerego
blakerego committed Mar 12, 2017
1 parent 80c5332 commit e78f133
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 40 deletions.
28 changes: 14 additions & 14 deletions README.md
View file
Expand Up @@ -66,10 +66,10 @@ directive.
*Example:*

```html
<masonry item-selector=".mybrick">
<div masonry item-selector=".mybrick">
<div masonry-brick class="mybrick">Unicorns</div>
<div masonry-brick class="mybrick">Sparkles</div>
</masonry>
</div>
```

### `column-width`
Expand All @@ -81,9 +81,9 @@ not set, Masonry will use the outer width of the first element.
*Example:*

```html
<masonry column-width="200">
<div masonry column-width="200">
<div class="masonry-brick">This will be 200px wide max.</div>
</masonry>
</div>
```

### `preserve-order`
Expand All @@ -96,10 +96,10 @@ elements isn't set at the time they are inserted.
*Example:*

```html
<masonry preserve-order>
<div masonry preserve-order>
<div class="masonry-brick"><img src="..." alt="Will always be shown 1st"></div>
<div class="masonry-brick"><img src="..." alt="Will always be shown 2nd"></div>
</masonry>
</div>
```

### `load-images`
Expand All @@ -109,10 +109,10 @@ Allows usage of `imagesLoaded` plugin. Defaults to `false`.
*Example:*

```html
<masonry load-images="true">
<div masonry load-images="true">
<div class="masonry-brick"><img src="/your/image_1.jpg" alt="image"/></div>
<div class="masonry-brick"><img src="/your/image_2.jpg" alt="image"/></div>
</masonry>
</div>
```

### `reload-on-show`
Expand All @@ -125,10 +125,10 @@ hidden it may not render properly when shown again.
*Example:*

```html
<masonry reload-on-show ng-show="showList">
<div masonry reload-on-show ng-show="showList">
<div class="masonry-brick">...</div>
<div class="masonry-brick">...</div>
</masonry>
</div>
```

When `showList` changes from falsey to truthy `ctrl.reload` will be called.
Expand All @@ -144,10 +144,10 @@ some blank space may be left on the sides.
*Example:*

```html
<masonry reload-on-resize>
<div masonry reload-on-resize>
<div class="masonry-brick">...</div>
<div class="masonry-brick">...</div>
</masonry>
</div>
```


Expand All @@ -159,8 +159,8 @@ as expression either as `masonry` or `masonry-options` attribute.
*Example:*

```html
<masonry masonry-options="{ transitionDuration: '0.4s' }">
</masonry>
<div masonry masonry-options="{ transitionDuration: '0.4s' }">
</div>
```

Equivalent to:
Expand Down
2 changes: 1 addition & 1 deletion src/angular-masonry.js
View file
Expand Up @@ -127,7 +127,7 @@

}).directive('masonry', function masonryDirective() {
return {
restrict: 'AE',
restrict: 'A',
controller: 'MasonryCtrl',
controllerAs: 'msnry',
link: {
Expand Down
50 changes: 25 additions & 25 deletions test/directive.coffee
View file
Expand Up @@ -22,28 +22,28 @@ describe 'angular-masonry', ->
window.Masonry = @Masonry

it 'should initialize', =>
@compile('<masonry></masonry>')(@scope)
@compile('<div masonry></div>')(@scope)

it 'should call masonry on init', =>
@compile('<div masonry></div>')(@scope)
expect(window.Masonry).toHaveBeenCalled()

it 'should pass on the column-width attribute', =>
@compile('<masonry column-width="200"></masonry>')(@scope)
@compile('<div masonry column-width="200"></div>')(@scope)
expect(window.Masonry).toHaveBeenCalledOnce()

call = window.Masonry.firstCall
expect(call.args[1].columnWidth).toBe 200

it 'should pass on the item-selector attribute', =>
@compile('<masonry item-selector=".mybrick"></masonry>')(@scope)
@compile('<div masonry item-selector=".mybrick"></div>')(@scope)
expect(window.Masonry).toHaveBeenCalled()

call = window.Masonry.firstCall
expect(call.args[1].itemSelector).toBe '.mybrick'

it 'should pass on any options provided via `masonry-options`', =>
@compile('<masonry masonry-options="{ isOriginLeft: true }"></masonry>')(@scope)
@compile('<div masonry masonry-options="{ isOriginLeft: true }"></div>')(@scope)
expect(window.Masonry).toHaveBeenCalled()

call = window.Masonry.firstCall
Expand All @@ -61,24 +61,24 @@ describe 'angular-masonry', ->
sinon.spy(@scope, '$watch')

it 'should setup a $watch when the reload-on-show is present', =>
@compile('<masonry reload-on-show></masonry>')(@scope)
@compile('<div masonry reload-on-show></div>')(@scope)
expect(@scope.$watch).toHaveBeenCalled()

it 'should not setup a $watch when the reload-on-show is missing', =>
@compile('<masonry></masonry>')(@scope)
@compile('<div masonry></div>')(@scope)
expect(@scope.$watch).not.toHaveBeenCalled()

it 'should setup a $watch when the reload-on-resize is present', =>
@compile('<masonry reload-on-resize></masonry>')(@scope)
@compile('<div masonry reload-on-resize></div>')(@scope)
expect(@scope.$watch).toHaveBeenCalledWith('masonryContainer.offsetWidth', sinon.match.func);

it 'should not setup a $watch when the reload-on-resize is missing', =>
@compile('<masonry></masonry>')(@scope)
@compile('<div masonry></div>')(@scope)
expect(@scope.$watch).not.toHaveBeenCalledWith('masonryContainer.offsetWidth', sinon.match.func);

describe 'MasonryCtrl', =>
beforeEach =>
@compile('<masonry></masonry>')(@scope)
@compile('<div masonry></div>')(@scope)
@ctrl = @scope.msnry
@ctrl.destroy()

Expand Down Expand Up @@ -107,9 +107,9 @@ describe 'angular-masonry', ->

it 'should register an element in the parent controller', =>
@compile('''
<masonry>
<div masonry>
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

expect(@scope.msnry.addBrick).toHaveBeenCalledOnce()
Expand All @@ -118,9 +118,9 @@ describe 'angular-masonry', ->
@scope.bricks = [1, 2, 3]

@compile('''
<masonry>
<div masonry>
<div class="masonry-brick" ng-repeat="brick in bricks"></div>
</masonry>
</div>
''')(@scope)

@scope.$digest() # Needed for initial ng-repeat
Expand All @@ -141,29 +141,29 @@ describe 'angular-masonry', ->

it 'should append three elements to the controller', =>
@compile('''
<masonry>
<div masonry>
<div class="masonry-brick"></div>
<div class="masonry-brick"></div>
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

expect(@scope.msnry.addBrick.callCount).toBe 3

it 'should prepend elements when specified by attribute', =>
@compile('''
<masonry>
<div masonry>
<div class="masonry-brick" prepend="{{true}}"></div>
</masonry>
</div>
''')(@scope)

expect(@scope.msnry.addBrick).toHaveBeenCalledWith 'prepended'

it 'should append before imagesLoaded when preserve-order is set', =>
@compile('''
<masonry load-images="true" preserve-order>
<div masonry load-images="true" preserve-order>
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

expect(@scope.msnry.addBrick).toHaveBeenCalledWith 'appended'
Expand All @@ -178,9 +178,9 @@ describe 'angular-masonry', ->
@spy = sinon.spy(window.Masonry.prototype, 'layout')

@compile('''
<masonry load-images="true" preserve-order>
<div masonry load-images="true" preserve-order>
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

@scope.$digest()
Expand All @@ -197,9 +197,9 @@ describe 'angular-masonry', ->
@spy = sinon.spy(window.Masonry.prototype, 'appended')

@compile('''
<masonry>
<div masonry>
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

expect(@spy).toHaveBeenCalledOnce()
Expand All @@ -210,9 +210,9 @@ describe 'angular-masonry', ->
@spy = sinon.spy(window.Masonry.prototype, 'layout');

@compile('''
<masonry load-images="false">
<div masonry load-images="false">
<div class="masonry-brick"></div>
</masonry>
</div>
''')(@scope)

@scope.$digest()
Expand Down

0 comments on commit e78f133

Please sign in to comment.