From 64c93843a88c7b8584c5a13261c8ecb9f1b137b7 Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Mon, 12 Oct 2015 12:10:20 -0700 Subject: [PATCH 1/5] Add the ability to mark cells. --- notebook/static/notebook/js/actions.js | 8 +++ notebook/static/notebook/js/cell.js | 44 ++++++++++++ .../static/notebook/js/keyboardmanager.js | 1 + notebook/static/notebook/js/notebook.js | 67 +++++++++++++++++++ notebook/static/notebook/less/cell.less | 5 ++ notebook/tests/notebook/marks.js | 56 ++++++++++++++++ 6 files changed, 181 insertions(+) create mode 100644 notebook/tests/notebook/marks.js diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index 292665a3c..efece6a69 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -423,6 +423,14 @@ define(function(require){ env.notebook.show_command_palette(); } }, + 'toggle-marks': { + help_index : 'cj', + help: 'toggle marks', + icon: 'fa-check', + handler : function(env){ + env.notebook.toggle_marks(env.notebook.get_selected_cells()); + } + }, }; /** diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 9a35a48fa..74861fa39 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -169,6 +169,11 @@ define([ if (!that.selected) { that.events.trigger('select.Cell', {'cell':that}); } + + // Ctrl-click should mark the c ell. + if (event.ctrlKey) { + that.marked = !that.marked; + } }); that.element.focusin(function (event) { if (!that.selected) { @@ -280,6 +285,45 @@ define([ } return was_selected_cell; }; + + /** + * Marks the cell + * @return {Cell} this + */ + Cell.prototype.mark = function() { + if (!this.marked) { + this.element.addClass('marked'); + } + return this; + }; + + /** + * Unmarks the cell + * @return {Cell} this + */ + Cell.prototype.unmark = function() { + if (this.marked) { + this.element.removeClass('marked'); + } + return this; + }; + + /** + * Whether or not the cell is marked. + * @return {boolean} + */ + Object.defineProperty(Cell.prototype, 'marked', { + get: function() { + return this.element.hasClass('marked'); + }, + set: function(value) { + if (value) { + this.mark(); + } else { + this.unmark(); + } + } + }); /** * should be overritten by subclass diff --git a/notebook/static/notebook/js/keyboardmanager.js b/notebook/static/notebook/js/keyboardmanager.js index 6663438b1..1fa1cf2bd 100644 --- a/notebook/static/notebook/js/keyboardmanager.js +++ b/notebook/static/notebook/js/keyboardmanager.js @@ -93,6 +93,7 @@ define([ 'enter' : 'ipython.enter-edit-mode', 'space' : 'ipython.scroll-down', 'down' : 'ipython.select-next-cell', + 'cmdtrl-space' : 'ipython.toggle-marks', 'i,i' : 'ipython.interrupt-kernel', '0,0' : 'ipython.restart-kernel', 'd,d' : 'ipython.delete-cell', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 92e3a0e0e..5859cce2d 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -628,6 +628,73 @@ define(function (require) { }); return result; }; + + /** + * Toggles the marks on the cells + * @param {Cell[]} [cells] - optionally specify what cells should be toggled + */ + Notebook.prototype.toggle_marks = function(cells) { + cells = cells || this.get_cells(); + cells.forEach(function(cell) { cell.marked = !cell.marked; }); + }; + + /** + * Mark all of the cells + * @param {Cell[]} [cells] - optionally specify what cells should be marked + */ + Notebook.prototype.mark_all = function(cells) { + cells = cells || this.get_cells(); + cells.forEach(function(cell) { cell.mark(); }); + }; + + /** + * Unmark all of the cells + * @param {Cell[]} [cells] - optionally specify what cells should be unmarked + */ + Notebook.prototype.unmark_all = function(cells) { + this.get_marked_cells(cells).forEach(function(cell) { cell.unmark(); }); + }; + + /** + * Set the cells that should be marked, exclusively + * @param {Cell[]} cells + */ + Notebook.prototype.set_marked_cells = function(cells) { + this.unmark_all(); + this.mark_all(cells); + }; + + /** + * Gets the cells that are marked + * @param {Cell[]} [cells] - optionally provide the cells to search through + * @return {Cell[]} marked cells + */ + Notebook.prototype.get_marked_cells = function(cells) { + cells = cells || this.get_cells(); + return cells.filter(function(cell) { return cell.marked; }); + }; + + /** + * Sets the cells that are marked by indices + * @param {number[]} indices + * @param {Cell[]} [cells] - optionally provide the cells to search through + */ + Notebook.prototype.set_marked_indices = function(indices, cells) { + cells = cells || this.get_cells(); + this.unmark_all(cells); + this.mark_all(cells.filter(function(cell, index) { return indices.indexOf(index) !== -1; })); + }; + + /** + * Gets the indices of the cells that are marked + * @param {Cell[]} [cells] - optionally provide the cells to search through + * @return {number[]} marked cell indices + */ + Notebook.prototype.get_marked_indices = function(cells) { + cells = cells || this.get_cells(); + var markedCells = this.get_marked_cells(cells); + return markedCells.map(function(cell) { return cells.indexOf(cell); }); + }; /** * Get an array of the cells in the currently selected range diff --git a/notebook/static/notebook/less/cell.less b/notebook/static/notebook/less/cell.less index d44d0928e..8e48cd322 100644 --- a/notebook/static/notebook/less/cell.less +++ b/notebook/static/notebook/less/cell.less @@ -21,6 +21,11 @@ div.cell { border-color: transparent; } } + + &.marked { + border-left-color: blue; + border-left-width: 3px; + } width: 100%; padding: 5px; diff --git a/notebook/tests/notebook/marks.js b/notebook/tests/notebook/marks.js new file mode 100644 index 000000000..32d2a5f2d --- /dev/null +++ b/notebook/tests/notebook/marks.js @@ -0,0 +1,56 @@ + +// Test +casper.notebook_test(function () { + var that = this; + + var a = 'print("a")'; + var index = this.append_cell(a); + + var b = 'print("b")'; + index = this.append_cell(b); + + var c = 'print("c")'; + index = this.append_cell(c); + + this.then(function () { + this.test.assertEquals(this.evaluate(function() { + return Jupyter.notebook.get_marked_cells().length; + }), 0, 'no cells are marked programmatically'); + + this.test.assertEquals(this.evaluate(function() { + return $('.cell.marked').length; + }), 0, 'no cells are marked visibily'); + + this.evaluate(function() { + Jupyter.notebook.mark_all(); + }); + + var cellCount = this.evaluate(function() { + return Jupyter.notebook.ncells(); + }); + + this.test.assertEquals(this.evaluate(function() { + return Jupyter.notebook.get_marked_cells().length; + }), cellCount, 'mark_all'); + + this.test.assertEquals(this.evaluate(function() { + return $('.cell.marked').length; + }), cellCount, 'marked cells are marked visibily'); + + this.evaluate(function() { + Jupyter.notebook.unmark_all(); + }); + + this.test.assertEquals(this.evaluate(function() { + return Jupyter.notebook.get_marked_cells().length; + }), 0, 'unmark_all'); + + this.evaluate(function() { + Jupyter.notebook.set_marked_indices([1]); + }); + + this.test.assertEquals(this.evaluate(function() { + return Jupyter.notebook.get_marked_indices()[0]; + }), 1, 'get/set_marked_indices'); + }); +}); From d934ac114cd5802ea257eb784f239ddcec0926ca Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Mon, 12 Oct 2015 13:18:40 -0700 Subject: [PATCH 2/5] Change hotkeys --- notebook/static/notebook/js/cell.js | 7 ++++--- notebook/static/notebook/js/keyboardmanager.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 74861fa39..7dc52332f 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -169,9 +169,10 @@ define([ if (!that.selected) { that.events.trigger('select.Cell', {'cell':that}); } - - // Ctrl-click should mark the c ell. - if (event.ctrlKey) { + + // Cmdtrl-click should mark the cell. + var isMac = navigator.platform.slice(0, 3).toLowerCase() === 'mac'; + if ((!isMac && event.ctrlKey) || (isMac && event.metaKey)) { that.marked = !that.marked; } }); diff --git a/notebook/static/notebook/js/keyboardmanager.js b/notebook/static/notebook/js/keyboardmanager.js index 1fa1cf2bd..519939cb6 100644 --- a/notebook/static/notebook/js/keyboardmanager.js +++ b/notebook/static/notebook/js/keyboardmanager.js @@ -93,7 +93,7 @@ define([ 'enter' : 'ipython.enter-edit-mode', 'space' : 'ipython.scroll-down', 'down' : 'ipython.select-next-cell', - 'cmdtrl-space' : 'ipython.toggle-marks', + 'ctrl-space' : 'ipython.toggle-marks', 'i,i' : 'ipython.interrupt-kernel', '0,0' : 'ipython.restart-kernel', 'd,d' : 'ipython.delete-cell', From 0c2cc3e695820921115febd7360bb1115a64f8ec Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Mon, 12 Oct 2015 14:57:16 -0700 Subject: [PATCH 3/5] Address @ellisonbg's comments --- notebook/static/notebook/js/actions.js | 4 +-- .../static/notebook/js/keyboardmanager.js | 2 +- notebook/static/notebook/js/notebook.js | 35 ++++++++++++++----- notebook/static/notebook/less/cell.less | 1 + notebook/tests/notebook/marks.js | 4 +-- 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index efece6a69..a169bcf80 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -423,12 +423,12 @@ define(function(require){ env.notebook.show_command_palette(); } }, - 'toggle-marks': { + 'toggle-cell-marked': { help_index : 'cj', help: 'toggle marks', icon: 'fa-check', handler : function(env){ - env.notebook.toggle_marks(env.notebook.get_selected_cells()); + env.notebook.toggle_cells_marked(env.notebook.get_selected_cells()); } }, }; diff --git a/notebook/static/notebook/js/keyboardmanager.js b/notebook/static/notebook/js/keyboardmanager.js index 519939cb6..407a7a11a 100644 --- a/notebook/static/notebook/js/keyboardmanager.js +++ b/notebook/static/notebook/js/keyboardmanager.js @@ -93,7 +93,7 @@ define([ 'enter' : 'ipython.enter-edit-mode', 'space' : 'ipython.scroll-down', 'down' : 'ipython.select-next-cell', - 'ctrl-space' : 'ipython.toggle-marks', + 'ctrl-space' : 'ipython.toggle-cell-marked', 'i,i' : 'ipython.interrupt-kernel', '0,0' : 'ipython.restart-kernel', 'd,d' : 'ipython.delete-cell', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 5859cce2d..9e060666d 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -401,7 +401,7 @@ define(function (require) { var st = sme.scrollTop(); var t = sme.offset().top; var ct = cells[index].element.offset().top; - var scroll_value = st + ct - (t + .01 * percent * h); + var scroll_value = st + ct - (t + 0.01 * percent * h); this.scroll_manager.element.animate({scrollTop:scroll_value}, time); return scroll_value; }; @@ -633,7 +633,7 @@ define(function (require) { * Toggles the marks on the cells * @param {Cell[]} [cells] - optionally specify what cells should be toggled */ - Notebook.prototype.toggle_marks = function(cells) { + Notebook.prototype.toggle_cells_marked = function(cells) { cells = cells || this.get_cells(); cells.forEach(function(cell) { cell.marked = !cell.marked; }); }; @@ -642,7 +642,7 @@ define(function (require) { * Mark all of the cells * @param {Cell[]} [cells] - optionally specify what cells should be marked */ - Notebook.prototype.mark_all = function(cells) { + Notebook.prototype.mark_all_cells = function(cells) { cells = cells || this.get_cells(); cells.forEach(function(cell) { cell.mark(); }); }; @@ -651,7 +651,7 @@ define(function (require) { * Unmark all of the cells * @param {Cell[]} [cells] - optionally specify what cells should be unmarked */ - Notebook.prototype.unmark_all = function(cells) { + Notebook.prototype.unmark_all_cells = function(cells) { this.get_marked_cells(cells).forEach(function(cell) { cell.unmark(); }); }; @@ -660,8 +660,8 @@ define(function (require) { * @param {Cell[]} cells */ Notebook.prototype.set_marked_cells = function(cells) { - this.unmark_all(); - this.mark_all(cells); + this.unmark_all_cells(); + this.mark_all_cells(cells); }; /** @@ -681,8 +681,8 @@ define(function (require) { */ Notebook.prototype.set_marked_indices = function(indices, cells) { cells = cells || this.get_cells(); - this.unmark_all(cells); - this.mark_all(cells.filter(function(cell, index) { return indices.indexOf(index) !== -1; })); + this.unmark_all_cells(cells); + this.mark_all_cells(cells.filter(function(cell, index) { return indices.indexOf(index) !== -1; })); }; /** @@ -695,6 +695,25 @@ define(function (require) { var markedCells = this.get_marked_cells(cells); return markedCells.map(function(cell) { return cells.indexOf(cell); }); }; + + /** + * Checks if the marked cells are contiguous + * @param {Cell[]} [cells] - optionally provide the cells to search through + * @return {boolean} + */ + Notebook.prototype.are_marks_contiguous = function(cells) { + // Get a numerically sorted list of the marked indices. + var markedIndices = this.get_marked_indices(cells).sort( + function(a,b) { return a-b; }); + + // Check for contiguousness + for (var i = 0; i < markedIndices.length - 1; i++) { + if (markedIndices[i+1] - markedIndices[i] !== 1) { + return false; + } + } + return true; + }; /** * Get an array of the cells in the currently selected range diff --git a/notebook/static/notebook/less/cell.less b/notebook/static/notebook/less/cell.less index 8e48cd322..ba10744ae 100644 --- a/notebook/static/notebook/less/cell.less +++ b/notebook/static/notebook/less/cell.less @@ -25,6 +25,7 @@ div.cell { &.marked { border-left-color: blue; border-left-width: 3px; + margin-left: -2px; } width: 100%; diff --git a/notebook/tests/notebook/marks.js b/notebook/tests/notebook/marks.js index 32d2a5f2d..bb7dc4dc6 100644 --- a/notebook/tests/notebook/marks.js +++ b/notebook/tests/notebook/marks.js @@ -22,7 +22,7 @@ casper.notebook_test(function () { }), 0, 'no cells are marked visibily'); this.evaluate(function() { - Jupyter.notebook.mark_all(); + Jupyter.notebook.mark_all_cells(); }); var cellCount = this.evaluate(function() { @@ -38,7 +38,7 @@ casper.notebook_test(function () { }), cellCount, 'marked cells are marked visibily'); this.evaluate(function() { - Jupyter.notebook.unmark_all(); + Jupyter.notebook.unmark_all_cells(); }); this.test.assertEquals(this.evaluate(function() { From 518e580af6cfecc4adbc190496989b15d774a670 Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Tue, 13 Oct 2015 11:52:10 -0700 Subject: [PATCH 4/5] Add some functions, remove some functions, and change the colors --- notebook/static/notebook/js/cell.js | 33 +++++--------------- notebook/static/notebook/js/notebook.js | 19 +++++++++-- notebook/static/notebook/less/cell.less | 4 ++- notebook/static/notebook/less/variables.less | 2 +- 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 7dc52332f..c6150a307 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -287,28 +287,6 @@ define([ return was_selected_cell; }; - /** - * Marks the cell - * @return {Cell} this - */ - Cell.prototype.mark = function() { - if (!this.marked) { - this.element.addClass('marked'); - } - return this; - }; - - /** - * Unmarks the cell - * @return {Cell} this - */ - Cell.prototype.unmark = function() { - if (this.marked) { - this.element.removeClass('marked'); - } - return this; - }; - /** * Whether or not the cell is marked. * @return {boolean} @@ -318,10 +296,13 @@ define([ return this.element.hasClass('marked'); }, set: function(value) { - if (value) { - this.mark(); - } else { - this.unmark(); + var isMarked = this.element.hasClass('marked'); + if (isMarked !== value) { + if (value) { + this.element.addClass('marked'); + } else { + this.element.removeClass('marked'); + } } } }); diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 9e060666d..fe2b4fc1e 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -644,7 +644,7 @@ define(function (require) { */ Notebook.prototype.mark_all_cells = function(cells) { cells = cells || this.get_cells(); - cells.forEach(function(cell) { cell.mark(); }); + cells.forEach(function(cell) { cell.marked = true; }); }; /** @@ -652,7 +652,7 @@ define(function (require) { * @param {Cell[]} [cells] - optionally specify what cells should be unmarked */ Notebook.prototype.unmark_all_cells = function(cells) { - this.get_marked_cells(cells).forEach(function(cell) { cell.unmark(); }); + this.get_marked_cells(cells).forEach(function(cell) { cell.marked = false; }); }; /** @@ -701,7 +701,7 @@ define(function (require) { * @param {Cell[]} [cells] - optionally provide the cells to search through * @return {boolean} */ - Notebook.prototype.are_marks_contiguous = function(cells) { + Notebook.prototype.are_marked_cells_contiguous = function(cells) { // Get a numerically sorted list of the marked indices. var markedIndices = this.get_marked_indices(cells).sort( function(a,b) { return a-b; }); @@ -714,6 +714,19 @@ define(function (require) { } return true; }; + + /** + * Checks if the marked cells specified by their indices are contiguous + * @param {number[]} indices - the cell indices to search through + * @param {Cell[]} [cells] - the cells to search through + * @return {boolean} + */ + Notebook.prototype.are_marked_indices_contiguous = function(indices, cells) { + cells = cells || this.get_cells(); + return this.are_marked_cells_contiguous(cells.filter(function(cell, index) { + return indices.indexOf(index) !== -1; + })); + }; /** * Get an array of the cells in the currently selected range diff --git a/notebook/static/notebook/less/cell.less b/notebook/static/notebook/less/cell.less index ba10744ae..1b389b8fa 100644 --- a/notebook/static/notebook/less/cell.less +++ b/notebook/static/notebook/less/cell.less @@ -23,9 +23,11 @@ div.cell { } &.marked { - border-left-color: blue; + border-left-color: #009AF5; border-left-width: 3px; margin-left: -2px; + border-bottom-left-radius: 0px; + border-top-left-radius: 0px; } width: 100%; diff --git a/notebook/static/notebook/less/variables.less b/notebook/static/notebook/less/variables.less index 76d56398f..f93bcaa68 100644 --- a/notebook/static/notebook/less/variables.less +++ b/notebook/static/notebook/less/variables.less @@ -11,7 +11,7 @@ @code_line_height: 1.21429em; // changed from 1.231 to get 17px even @code_padding: 0.4em; // 5.6 px @rendered_html_border_color: black; -@input_prompt_color: navy; +@input_prompt_color: #2F97C1; @output_prompt_color: darkred; @output_pre_color: black; @notification_widget_bg: rgba(240, 240, 240, 0.5); From f86888bfc14a3d49789e4b72330e3d7454d177d6 Mon Sep 17 00:00:00 2001 From: Jonathan Frederic Date: Wed, 14 Oct 2015 12:19:56 -0700 Subject: [PATCH 5/5] Dummy commit --- notebook/static/notebook/less/cell.less | 1 + 1 file changed, 1 insertion(+) diff --git a/notebook/static/notebook/less/cell.less b/notebook/static/notebook/less/cell.less index 1b389b8fa..888ff4cc8 100644 --- a/notebook/static/notebook/less/cell.less +++ b/notebook/static/notebook/less/cell.less @@ -16,6 +16,7 @@ div.cell { .edit_mode &.selected { border-color: green; + /* Don't border the cells when printing */ @media print { border-color: transparent;