Merge pull request #331 from jods4/issue-328

fix(if): nested ifs don't work properly

src/else.js

@@ -14,7 +14,9 @@ export class Else extends IfCore {
   bind(bindingContext, overrideContext) {
     super.bind(bindingContext, overrideContext);
     // Render on initial
-    if (!this.ifVm.condition) {
+    if (this.ifVm.condition) {
+      this._hide();
+    } else {
       this._show();
     }
   }

src/if-core.js

@@ -31,7 +31,6 @@ export class IfCore {
     // For example a view could be returned to the cache and reused while it's still
     // attached to the DOM and animated.
     if (!this.viewFactory.isCaching) {
-	    this.showing = false;
       return;
     }
 
@@ -47,6 +46,12 @@ export class IfCore {
 
   _show() {
     if (this.showing) {
+      // Ensures the view is bound.
+      // It might not be the case when the if was unbound but not detached, then rebound.
+      // Typical case where this happens is nested ifs
+      if (!this.view.isBound) {
+        this.view.bind(this.bindingContext, this.overrideContext);
+      }
       return;
     }
 

src/if.js

@@ -22,6 +22,8 @@ export class If extends IfCore {
     super.bind(bindingContext, overrideContext);
     if (this.condition) {
       this._show();
+    } else {
+      this._hide();
     }
   }
 

test/else.spec.js

@@ -136,4 +136,22 @@ describe('else', () => {
     expect(elseViewSlot.remove).toHaveBeenCalledTimes(1);
     expect(elseVm.view.unbind).toHaveBeenCalledTimes(1);
   });
+
+  it('should re-bind the child-view when being bound itself and condition is falsy', () => {
+    ifVm.condition = false;
+    elseVm.showing = true;
+    elseVm.view = new ViewMock();
+
+    spyOn(elseVm.view, 'bind');
+
+    let bindingContext = 42;
+    let overrideContext = 24;
+
+    spyOn(elseVm, '_show').and.callThrough();
+
+    elseVm.bind(bindingContext, overrideContext);
+
+    expect(elseVm._show).toHaveBeenCalled();
+    expect(elseVm.view.bind).toHaveBeenCalled();
+  });
 });

test/if-test.js

@@ -0,0 +1,11 @@
+import {BoundViewFactory, ViewSlot, bindable, customAttribute, templateController} from 'aurelia-templating';
+import {If} from '../src/if';
+import {inject} from 'aurelia-dependency-injection';
+
+@customAttribute('ift')
+@templateController
+@inject(BoundViewFactory, ViewSlot)
+export class IfTest extends If {
+  @bindable({primaryProperty: true}) condition;
+  @bindable swapOrder;
+}

test/if.spec.js

@@ -2,6 +2,8 @@ import './setup';
 import {If} from '../src/if';
 import {BoundViewFactory, ViewSlot, View} from 'aurelia-templating';
 import {TaskQueue} from 'aurelia-task-queue';
+import {bootstrap} from 'aurelia-bootstrapper';
+import {ComponentTester} from 'aurelia-testing';
 
 describe('if', () => {
   let viewSlot, taskQueue, sut, viewFactory;
@@ -75,7 +77,7 @@ describe('if', () => {
     sut.unbind();
     taskQueue.flushMicroTaskQueue();
 
-    expect(sut.showing).toBeFalsy();
+    expect(sut.showing).toBeTruthy();
     expect(view.unbind).toHaveBeenCalled();
     expect(viewSlot.remove).not.toHaveBeenCalled();
     expect(view.returnToCache).not.toHaveBeenCalled();
@@ -122,6 +124,36 @@ describe('if', () => {
     expect(newView.bind).toHaveBeenCalledWith(42, 24);
   });
 
+  it('should rebind child-view if needed when being bound itself and condition is truthy', () => {
+    sut.condition = true;
+    sut.showing = true;
+    sut.view = {isBound: false, bind: jasmine.createSpy('bind')};
+    let bindingContext = 42;
+    let overrideContext = 24;
+
+    spyOn(sut, '_show').and.callThrough();
+
+    sut.bind(bindingContext, overrideContext);
+
+    expect(sut._show).toHaveBeenCalled();
+    expect(sut.view.bind).toHaveBeenCalledWith(42, 24);
+  });
+
+  it('should unbind the child-view when being bound itself and condition is falsy', () => {
+    sut.condition = false;
+    sut.showing = true;
+    sut.view = {isBound: false, unbind: jasmine.createSpy('unbind')};
+    let bindingContext = 42;
+    let overrideContext = 24;
+
+    spyOn(sut, '_hide').and.callThrough();
+
+    sut.bind(bindingContext, overrideContext);
+
+    expect(sut._hide).toHaveBeenCalled();
+    expect(sut.view.unbind).toHaveBeenCalled();
+  });
+
   it('should show the view when provided value is truthy and currently not showing', () => {
     sut.showing = false;
     sut.view = new ViewMock();
@@ -188,14 +220,85 @@ describe('if', () => {
       }).then(() => {
         expect(viewSlot.children.length).toEqual(1);
       })
-      .then(() => done())
-    })
+      .then(() => done());
+    });
+  });
+
+  xdescribe('in a nested situation', () => {
+    let component;
+    let model;
+
+    beforeEach(() => {
+      model = { a: true, b: true, clicked() { } };
+      component = new ComponentTester();
+      component.bootstrap(aurelia => {
+        aurelia.use.standardConfiguration();
+        taskQueue = aurelia.container.get(TaskQueue);
+      });
+      component
+        .withResources('test/if-test')
+        .inView(`<div ift.bind="a" id="a">
+                  <div ift.bind="b" id="b" click.delegate="clicked()">
+                  </div>
+                </div>`)
+        .boundTo(model);
+    });
+
+    afterEach(() => component.dispose());
+
+    it('should rebind even if the view is visible; issue #317', done => {
+      let spy;
+      component.create(bootstrap)
+        .then(_ => {
+          spy = spyOn(model, 'clicked');
+          model.a = false;
+          taskQueue.flushMicroTaskQueue();
+
+          // This is horrible, but the only way I found to get things working
+          return new Promise(r => setTimeout(r, 1));
+        }).then(() => {
+          model.a = true;
+          taskQueue.flushMicroTaskQueue();
+          return new Promise(r => setTimeout(r, 1));
+        }).then(() => {
+          document.getElementById('b').click();
+
+          expect(spy).toHaveBeenCalled();
+          done();
+        })
+        .catch(e => {
+          fail(e);
+          done();
+        });
+    });
+
+    it('should update view when rebound; issue #328', done => {
+      component.create(bootstrap)
+        .then(_ => {
+          model.a = false;
+          taskQueue.flushMicroTaskQueue();
+          return new Promise(r => setTimeout(r, 1));
+        }).then(() => {
+          model.b = false;
+          model.a = true;
+          taskQueue.flushMicroTaskQueue();
+          return new Promise(r => setTimeout(r, 1));
+        }).then(() => {
+          expect(document.getElementById('a')).not.toBeNull();
+          expect(document.getElementById('b')).toBeNull();
+          done();
+        })
+        .catch(e => {
+          fail(e);
+          done();
+        });
+    });
   });
 });
 
 class ViewSlotMock {
   remove() {}
-  add () {}
+  add() {}
 }
 
 class ViewMock {

test/repeat.spec.js

@@ -69,7 +69,7 @@ describe('repeat', () => {
       spyOn(viewSlot, 'removeAll');
       repeat.unbind();
 
-      expect(viewSlot.removeAll).toHaveBeenCalledWith(true);
+      expect(viewSlot.removeAll).toHaveBeenCalledWith(true, true);
     });
 
     it('should unsubscribe collection', () => {