Merge pull request #105 from blink1073/toolbar-test

Toolbar cleanup and tests

src/notebook/notebook/default-toolbar.ts

@@ -102,9 +102,11 @@ namespace ToolbarItems {
    */
   export
   function createSaveButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_SAVE, () => {
-      panel.context.save();
-    }, 'Save the notebook contents');
+    return new ToolbarButton({
+      className: TOOLBAR_SAVE,
+      onClick: () => { panel.context.save();  },
+      tooltip: 'Save the notebook contents'
+    });
   }
 
   /**
@@ -112,9 +114,11 @@ namespace ToolbarItems {
    */
   export
   function createInsertButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_INSERT, () => {
-      NotebookActions.insertBelow(panel.content);
-    }, 'Insert a cell below');
+    return new ToolbarButton({
+      className: TOOLBAR_INSERT,
+      onClick: () => { NotebookActions.insertBelow(panel.content); },
+      tooltip: 'Insert a cell below'
+    });
   }
 
   /**
@@ -122,9 +126,13 @@ namespace ToolbarItems {
    */
   export
   function createCutButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_CUT, () => {
-      NotebookActions.cut(panel.content, panel.clipboard);
-    }, 'Cut the selected cell(s)');
+    return new ToolbarButton({
+      className: TOOLBAR_CUT,
+      onClick: () => {
+        NotebookActions.cut(panel.content, panel.clipboard);
+      },
+      tooltip: 'Cut the selected cell(s)'
+    });
   }
 
   /**
@@ -132,9 +140,13 @@ namespace ToolbarItems {
    */
   export
   function createCopyButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_COPY, () => {
-      NotebookActions.copy(panel.content, panel.clipboard);
-    }, 'Copy the selected cell(s)');
+    return new ToolbarButton({
+      className: TOOLBAR_COPY,
+      onClick: () => {
+        NotebookActions.copy(panel.content, panel.clipboard);
+      },
+      tooltip: 'Copy the selected cell(s)'
+    });
   }
 
   /**
@@ -142,9 +154,13 @@ namespace ToolbarItems {
    */
   export
   function createPasteButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_PASTE, () => {
-      NotebookActions.paste(panel.content, panel.clipboard);
-    }, 'Paste cell(s) from the clipboard');
+    return new ToolbarButton({
+      className: TOOLBAR_PASTE,
+      onClick: () => {
+        NotebookActions.paste(panel.content, panel.clipboard);
+      },
+      tooltip: 'Paste cell(s) from the clipboard'
+    });
   }
 
   /**
@@ -152,9 +168,13 @@ namespace ToolbarItems {
    */
   export
   function createRunButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_RUN, () => {
-      NotebookActions.runAndAdvance(panel.content, panel.context.kernel);
-    }, 'Run the selected cell(s) and advance');
+    return new ToolbarButton({
+      className: TOOLBAR_RUN,
+      onClick: () => {
+        NotebookActions.runAndAdvance(panel.content, panel.context.kernel);
+      },
+      tooltip: 'Run the selected cell(s) and advance'
+    });
   }
 
   /**
@@ -162,11 +182,15 @@ namespace ToolbarItems {
    */
   export
   function createInterruptButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_INTERRUPT, () => {
-      if (panel.context.kernel) {
-        panel.context.kernel.interrupt();
-      }
-    }, 'Interrupt the kernel');
+    return new ToolbarButton({
+      className: TOOLBAR_INTERRUPT,
+      onClick: () => {
+        if (panel.context.kernel) {
+          panel.context.kernel.interrupt();
+        }
+      },
+      tooltip: 'Interrupt the kernel'
+    });
   }
 
   /**
@@ -174,9 +198,13 @@ namespace ToolbarItems {
    */
   export
   function createRestartButton(panel: NotebookPanel): ToolbarButton {
-    return new ToolbarButton(TOOLBAR_RESTART, () => {
-      panel.restart();
-    }, 'Restart the kernel');
+    return new ToolbarButton({
+      className: TOOLBAR_RESTART,
+      onClick: () => {
+        panel.restart();
+      },
+      tooltip: 'Restart the kernel'
+    });
   }
 
   /**

src/notebook/notebook/toolbar.ts

@@ -77,13 +77,13 @@ class NotebookToolbar extends Widget {
     if (index === -1) {
       layout.addChild(widget);
     } else {
-      layout.insertChild(index, widget);
+      layout.insertChild(index + 1, widget);
     }
     Private.nameProperty.set(widget, name);
   }
 
   /**
-   * List the names of the toolbar items.
+   * Get an ordered list the toolbar item names.
    *
    * @returns A new array of the current toolbar item names.
    */
@@ -114,14 +114,15 @@ class ToolbarButton extends Widget {
   /**
    * Construct a new toolbar button.
    */
-  constructor(className: string, onClick: () => void, tooltip?: string) {
+  constructor(options: ToolbarButton.IOptions = {}) {
     super();
+    options = options || {};
     this.addClass(TOOLBAR_BUTTON);
-    this.addClass(className);
-    this._onClick = onClick;
-    if (tooltip) {
-      this.node.title = tooltip;
+    this._onClick = options.onClick;
+    if (options.className) {
+      this.addClass(options.className);
     }
+    this.node.title = options.tooltip || '';
   }
 
   /**
@@ -146,7 +147,9 @@ class ToolbarButton extends Widget {
     switch (event.type) {
     case 'click':
       let cb = this._onClick;
-      cb();
+      if (cb) {
+        cb();
+      }
       break;
     case 'mousedown':
       this.addClass(TOOLBAR_PRESSED);
@@ -180,7 +183,35 @@ class ToolbarButton extends Widget {
     this.node.removeEventListener('mouseout', this);
   }
 
-  private _onClick: () => void = null;
+  private _onClick: () => void;
+}
+
+
+/**
+ * A namespace for `ToolbarButton` statics.
+ */
+export
+namespace ToolbarButton {
+  /**
+   * The options used to construct a toolbar button.
+   */
+  export
+  interface IOptions {
+    /**
+     * The callback for a click event.
+     */
+    onClick?: () => void;
+
+    /**
+     * The class name added to the button.
+     */
+    className?: string;
+
+    /**
+     * The tooltip added to the button node.
+     */
+    tooltip?: string;
+  }
 }
 
 

test/src/index.ts

@@ -10,6 +10,7 @@ import './renderers/latex.spec';
 import './notebook/notebook/nbformat.spec';
 import './notebook/notebook/model.spec';
 import './notebook/notebook/modelfactory.spec';
+import './notebook/notebook/toolbar.spec';
 import './notebook/notebook/widget.spec';
 import './notebook/notebook/widgetfactory.spec';
 

test/src/notebook/notebook/toolbar.spec.ts

@@ -0,0 +1,270 @@
+// Copyright (c) Jupyter Development Team.
+// Distributed under the terms of the Modified BSD License.
+
+import expect = require('expect.js');
+
+import {
+  Message
+} from 'phosphor-messaging';
+
+import {
+  Widget
+} from 'phosphor-widget';
+
+import {
+  simulate
+} from 'simulate-event';
+
+import {
+  NotebookToolbar, ToolbarButton
+} from '../../../../lib/notebook/notebook/toolbar';
+
+
+class LogToolbarButton extends ToolbarButton {
+
+  events: string[] = [];
+
+  methods: string[] = [];
+
+  handleEvent(event: Event): void {
+    super.handleEvent(event);
+    this.events.push(event.type);
+  }
+
+  protected onAfterAttach(msg: Message): void {
+    super.onAfterAttach(msg);
+    this.methods.push('onAfterAttach');
+  }
+
+  protected onBeforeDetach(msg: Message): void {
+    super.onBeforeDetach(msg);
+    this.methods.push('onBeforeDetach');
+  }
+}
+
+
+describe('notebook/notebook/toolbar', () => {
+
+  describe('NotebookToolbar', () => {
+
+    describe('#constructor()', () => {
+
+      it('should construct a new toolbar widget', () => {
+        let widget = new NotebookToolbar();
+        expect(widget).to.be.a(NotebookToolbar);
+      });
+
+      it('should add the `jp-NBToolbar` class', () => {
+        let widget = new NotebookToolbar();
+        expect(widget.hasClass('jp-NBToolbar')).to.be(true);
+      });
+
+    });
+
+    describe('#add()', () => {
+
+      it('should add an item to the toolbar', () => {
+        let item = new Widget();
+        let widget = new NotebookToolbar();
+        widget.add('test', item);
+        expect(widget.list()).to.contain('test');
+      });
+
+      it('should add the `jp-NBToolbar-item` class to the widget', () => {
+        let item = new Widget();
+        let widget = new NotebookToolbar();
+        widget.add('test', item);
+        expect(item.hasClass('jp-NBToolbar-item')).to.be(true);
+      });
+
+      it('should add the item after a named item', () => {
+        let items = [new Widget(), new Widget(), new Widget()];
+        let widget = new NotebookToolbar();
+        widget.add('foo', items[0]);
+        widget.add('bar', items[1]);
+        widget.add('baz', items[2], 'foo');
+        expect(widget.list()).to.eql(['foo', 'baz', 'bar']);
+      });
+
+      it('should ignore an invalid `after`', () => {
+        let items = [new Widget(), new Widget(), new Widget()];
+        let widget = new NotebookToolbar();
+        widget.add('foo', items[0]);
+        widget.add('bar', items[1]);
+        widget.add('baz', items[2], 'fuzz');
+        expect(widget.list()).to.eql(['foo', 'bar', 'baz']);
+      });
+
+      it('should throw an error if the name is alreay used', () => {
+        let widget = new NotebookToolbar();
+        widget.add('test', new Widget());
+        expect(() => { widget.add('test', new Widget()); }).to.throwError();
+      });
+
+    });
+
+    describe('#list()', () => {
+
+      it('should get an ordered list the toolbar item names', () => {
+        let widget = new NotebookToolbar();
+        widget.add('foo', new Widget());
+        widget.add('bar', new Widget());
+        widget.add('baz', new Widget());
+        expect(widget.list()).to.eql(['foo', 'bar', 'baz']);
+      });
+
+    });
+
+  });
+
+  describe('ToolbarButton', () => {
+
+    describe('#createNode()', () => {
+
+      it('should create a node for the toolbar button', () => {
+        let node = ToolbarButton.createNode();
+        expect(node.localName).to.be('span');
+      });
+
+    });
+
+    describe('#constructor()', () => {
+
+      it('should accept no arguments', () => {
+        let button = new ToolbarButton();
+        expect(button).to.be.a(ToolbarButton);
+      });
+
+      it('should accept options', () => {
+        let button = new ToolbarButton({
+          className: 'foo',
+          onClick: () => { return void 0; },
+          tooltip: 'bar'
+        });
+        expect(button.hasClass('foo')).to.be(true);
+        expect(button.node.title).to.be('bar');
+      });
+
+    });
+
+    describe('#dispose()', () => {
+
+      it('should dispose of the resources used by the widget', () => {
+        let button = new ToolbarButton();
+        button.dispose();
+        expect(button.isDisposed).to.be(true);
+      });
+
+      it('should be safe to call more than once', () => {
+        let button = new ToolbarButton();
+        button.dispose();
+        button.dispose();
+        expect(button.isDisposed).to.be(true);
+      });
+
+    });
+
+    describe('#handleEvent()', () => {
+
+      context('click', () => {
+
+        it('should activate the callback', (done) => {
+          let called = false;
+          let button = new ToolbarButton({
+            onClick: () => { called = true; },
+          });
+          button.attach(document.body);
+          requestAnimationFrame(() => {
+            simulate(button.node, 'click');
+            expect(called).to.be(true);
+            done();
+          });
+        });
+
+      });
+
+      context('mousedown', () => {
+
+        it('should add the `jp-mod-pressed` class', (done) => {
+          let button = new ToolbarButton();
+          button.attach(document.body);
+          requestAnimationFrame(() => {
+            simulate(button.node, 'mousedown');
+            expect(button.hasClass('jp-mod-pressed')).to.be(true);
+            done();
+          });
+        });
+
+      });
+
+      context('mouseup', () => {
+
+        it('should remove the `jp-mod-pressed` class', (done) => {
+          let button = new ToolbarButton();
+          button.attach(document.body);
+          requestAnimationFrame(() => {
+            simulate(button.node, 'mousedown');
+            simulate(button.node, 'mouseup');
+            expect(button.hasClass('jp-mod-pressed')).to.be(false);
+            done();
+          });
+        });
+
+      });
+
+      context('mouseout', () => {
+
+        it('should remove the `jp-mod-pressed` class', (done) => {
+          let button = new ToolbarButton();
+          button.attach(document.body);
+          requestAnimationFrame(() => {
+            simulate(button.node, 'mousedown');
+            simulate(button.node, 'mouseout');
+            expect(button.hasClass('jp-mod-pressed')).to.be(false);
+            done();
+          });
+        });
+
+      });
+
+    });
+
+    describe('#onAfterAttach()', () => {
+
+      it('should add event listeners to the node', () => {
+        let button = new LogToolbarButton();
+        button.attach(document.body);
+        expect(button.methods).to.contain('onAfterAttach');
+        simulate(button.node, 'mousedown');
+        simulate(button.node, 'mouseup');
+        simulate(button.node, 'mouseout');
+        expect(button.events).to.contain('mousedown');
+        expect(button.events).to.contain('mouseup');
+        expect(button.events).to.contain('mouseout');
+      });
+
+    });
+
+    describe('#onBeforeDetach()', () => {
+
+      it('should remove event listeners from the node', (done) => {
+        let button = new LogToolbarButton();
+        button.attach(document.body);
+        requestAnimationFrame(() => {
+          button.detach();
+          expect(button.methods).to.contain('onBeforeDetach');
+          simulate(button.node, 'mousedown');
+          simulate(button.node, 'mouseup');
+          simulate(button.node, 'mouseout');
+          expect(button.events).to.not.contain('mousedown');
+          expect(button.events).to.not.contain('mouseup');
+          expect(button.events).to.not.contain('mouseout');
+          done();
+        });
+      });
+
+    });
+
+  });
+
+});