Merge pull request #1105 from afshin/notebook-tracker

Notebook tracker tests and bug fixes.

src/notebook/tracker.ts

@@ -90,17 +90,25 @@ class NotebookTracker extends InstanceTracker<NotebookPanel> implements INoteboo
     if (this.isDisposed) {
       return;
     }
-    super.dispose();
     if (this._handler) {
       this._handler.dispose();
       this._handler = null;
     }
+    this._activeCell = null;
+    super.dispose();
   }
 
   /**
    * Handle the current change event.
    */
   protected onCurrentChanged(): void {
+    // Store an internal reference to active cell to prevent false positives.
+    let activeCell = this.activeCell;
+    if (activeCell && activeCell === this._activeCell) {
+      return;
+    }
+    this._activeCell = activeCell;
+
     if (this._handler) {
       this._handler.dispose();
     }
@@ -110,15 +118,25 @@ class NotebookTracker extends InstanceTracker<NotebookPanel> implements INoteboo
       return;
     }
 
+    // Create a signal handler for cell changes.
     let changeHandler = (sender: any, cell: BaseCellWidget) => {
       this.activeCellChanged.emit(cell || null);
     };
+
+    // Connect the signal handler to the current notebook panel.
     widget.content.activeCellChanged.connect(changeHandler);
     this._handler = new DisposableDelegate(() => {
-      widget.content.activeCellChanged.disconnect(changeHandler);
+      // Only disconnect if the widget still exists.
+      if (!widget.isDisposed) {
+        widget.content.activeCellChanged.disconnect(changeHandler);
+      }
     });
+
+    // Since the notebook has changed, immediately signal an active cell change.
+    this.activeCellChanged.emit(widget.content.activeCell || null);
   }
 
+  private _activeCell: BaseCellWidget = null;
   private _handler: DisposableDelegate = null;
 }
 

test/src/index.ts

@@ -47,6 +47,8 @@ import './notebook/notebook/widgetfactory.spec';
 import './notebook/output-area/model.spec';
 import './notebook/output-area/widget.spec';
 
+import './notebook/tracker.spec';
+
 import './toolbar/toolbar.spec';
 
 import 'phosphor/styles/base.css';

test/src/notebook/notebook/panel.spec.ts

@@ -118,7 +118,9 @@ describe('notebook/notebook/panel', () => {
 
       it('should accept an optional render', () => {
         let newRenderer = new CodeMirrorNotebookPanelRenderer();
-        let panel = new NotebookPanel({ rendermime, clipboard, renderer: newRenderer});
+        let panel = new NotebookPanel({
+          rendermime, clipboard, renderer: newRenderer
+        });
         expect(panel.renderer).to.be(newRenderer);
       });
 

test/src/notebook/tracker.spec.ts

@@ -0,0 +1,140 @@
+// Copyright (c) Jupyter Development Team.
+// Distributed under the terms of the Modified BSD License.
+
+import expect = require('expect.js');
+
+import {
+  MimeData
+} from 'phosphor/lib/core/mimedata';
+
+import {
+  BaseCellWidget
+} from '../../../lib/notebook/cells';
+
+import {
+  CodeMirrorNotebookPanelRenderer
+} from '../../../lib/notebook/codemirror/notebook/panel';
+
+import {
+  NotebookPanel
+} from '../../../lib/notebook/notebook/panel';
+
+import {
+  NotebookTracker
+} from '../../../lib/notebook/tracker';
+
+import {
+  createNotebookContext, defaultRenderMime
+} from '../utils';
+
+import {
+  DEFAULT_CONTENT
+} from './utils';
+
+
+class TestTracker extends NotebookTracker {
+  methods: string[] = [];
+
+  protected onCurrentChanged(): void {
+    super.onCurrentChanged();
+    this.methods.push('onCurrentChanged');
+  }
+}
+
+
+/**
+ * Default notebook panel data.
+ */
+const rendermime = defaultRenderMime();
+const clipboard = new MimeData();
+const renderer = CodeMirrorNotebookPanelRenderer.defaultRenderer;
+
+
+describe('notebook/tracker', () => {
+
+  describe('NotebookTracker', () => {
+
+    describe('#constructor()', () => {
+
+      it('should create a NotebookTracker', () => {
+        let tracker = new NotebookTracker();
+        expect(tracker).to.be.a(NotebookTracker);
+      });
+
+    });
+
+    describe('#activeCell', () => {
+
+      it('should be `null` if there is no tracked notebook panel', () => {
+        let tracker = new NotebookTracker();
+        expect(tracker.activeCell).to.be(null);
+      });
+
+      it('should be `null` if a tracked notebook has no active cell', () => {
+        let tracker = new NotebookTracker();
+        let panel = new NotebookPanel({ rendermime, clipboard, renderer});
+        tracker.add(panel);
+        tracker.sync(panel);
+        expect(tracker.activeCell).to.be(null);
+      });
+
+      it('should be the active cell if a tracked notebook has one', (done) => {
+        let tracker = new NotebookTracker();
+        let panel = new NotebookPanel({ rendermime, clipboard, renderer});
+        tracker.add(panel);
+        tracker.sync(panel);
+        createNotebookContext().then(context => {
+          panel.context = context;
+          panel.content.model.fromJSON(DEFAULT_CONTENT);
+          expect(tracker.activeCell).to.be.a(BaseCellWidget);
+          panel.dispose();
+          done();
+        }).catch(done);
+      });
+
+    });
+
+    describe('#activeCellChanged', () => {
+
+      it('should emit a signal when the active cell changes', (done) => {
+        let tracker = new NotebookTracker();
+        let panel = new NotebookPanel({ rendermime, clipboard, renderer });
+        let count = 0;
+        tracker.activeCellChanged.connect(() => { count++; });
+        createNotebookContext().then(context => {
+          panel.context = context;
+          panel.content.model.fromJSON(DEFAULT_CONTENT);
+          expect(count).to.be(0);
+          tracker.add(panel);
+          tracker.sync(panel);
+          expect(count).to.be(1);
+          panel.content.activeCellIndex = 1;
+          expect(count).to.be(2);
+          panel.dispose();
+          done();
+        }).catch(done);
+      });
+
+    });
+
+    describe('#onCurrentChanged()', () => {
+
+      it('should be called when the active cell changes', (done) => {
+        let tracker = new TestTracker();
+        let panel = new NotebookPanel({ rendermime, clipboard, renderer});
+        tracker.add(panel);
+        tracker.sync(panel);
+        createNotebookContext().then(context => {
+          panel.context = context;
+          panel.content.model.fromJSON(DEFAULT_CONTENT);
+          expect(tracker.methods).to.contain('onCurrentChanged');
+          panel.dispose();
+          done();
+        }).catch(done);
+      });
+
+    });
+
+  });
+
+});