From 0376350e266322b5c0779c3cc2842527ed81117e Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Tue, 11 Jul 2017 07:12:34 +0100 Subject: [PATCH 01/24] Allow none in accordion widget --- .../widgets/widget_selectioncontainer.py | 2 +- .../controls/src/phosphor/currentselection.ts | 40 +++++++++++-------- .../controls/src/widget_selectioncontainer.ts | 2 +- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/ipywidgets/widgets/widget_selectioncontainer.py b/ipywidgets/widgets/widget_selectioncontainer.py index e536cffe9d..404005ef30 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -16,7 +16,7 @@ class _SelectionContainer(Box, CoreWidget): """Base class used to display multiple child widgets.""" _titles = Dict(help="Titles of the pages").tag(sync=True) - selected_index = CInt(help="The index of the selected page.").tag(sync=True) + selected_index = CInt(help="The index of the selected page.", allow_none=True).tag(sync=True) # Public methods def set_title(self, index, title): diff --git a/packages/controls/src/phosphor/currentselection.ts b/packages/controls/src/phosphor/currentselection.ts index 0cf9af0301..ce9c24df60 100644 --- a/packages/controls/src/phosphor/currentselection.ts +++ b/packages/controls/src/phosphor/currentselection.ts @@ -93,7 +93,7 @@ class Selection { */ set value(value: T) { if (value === null) { - this.index = -1; + this.index = null; } else { this.index = ArrayExt.firstIndexOf(this._array, value); } @@ -103,9 +103,9 @@ class Selection { * Get the index of the currently selected item. * * #### Notes - * This will be `-1` if no item is selected. + * This will be `null` if no item is selected. */ - get index(): number { + get index(): number | null { return this._index; } @@ -115,14 +115,20 @@ class Selection { * @param index - The index to select. * * #### Notes - * If the value is out of range, the index will be set to `-1`, which + * If the value is out of range, the index will be set to `null`, which * indicates no item is selected. */ - set index(index: number) { + set index(index: number | null) { // Coerce the value to an index. - let i = Math.floor(index); - if (i < 0 || i >= this._array.length) { - i = -1; + let i; + if (index !== null) { + i = Math.floor(index); + if (i < 0 || i >= this._array.length) { + i = null; + } + } + else { + i = null; } // Bail early if the index will not change. @@ -194,7 +200,7 @@ class Selection { // Handle the behavior where the new item is always selected, // or the behavior where the new item is selected if needed. - if (bh === 'select-item' || (bh === 'select-item-if-needed' && ci === -1)) { + if (bh === 'select-item' || (bh === 'select-item-if-needed' && ci === null)) { this._index = i; this._value = item; this._previousValue = cv; @@ -240,19 +246,19 @@ class Selection { let pv = this._value; // Reset the current index and previous item. - this._index = -1; + this._index = null; this._value = null; this._previousValue = null; // If no item was selected, there's nothing else to do. - if (pi === -1) { + if (pi === null) { return; } // Emit the current changed signal. this._selectionChanged.emit({ previousIndex: pi, previousValue: pv, - currentIndex: -1, currentValue: null + currentIndex: this._index, currentValue: this._value }); } @@ -283,12 +289,12 @@ class Selection { // No item gets selected if the vector is empty. if (this._array.length === 0) { // Reset the current index and previous item. - this._index = -1; + this._index = null; this._value = null; this._previousValue = null; this._selectionChanged.emit({ previousIndex: i, previousValue: item, - currentIndex: -1, currentValue: null + currentIndex: this._index, currentValue: this._value }); return; } @@ -334,12 +340,12 @@ class Selection { } // Otherwise, no item gets selected. - this._index = -1; + this._index = null; this._value = null; this._previousValue = null; this._selectionChanged.emit({ previousIndex: i, previousValue: item, - currentIndex: -1, currentValue: null + currentIndex: this._index, currentValue: this._value }); } @@ -348,7 +354,7 @@ class Selection { */ private _updateSelectedValue() { let i = this._index; - this._value = i !== -1 ? this._array[i] : null; + this._value = i !== null ? this._array[i] : null; } private _array: ReadonlyArray = null; diff --git a/packages/controls/src/widget_selectioncontainer.ts b/packages/controls/src/widget_selectioncontainer.ts index 54e84ba6ca..55f26c5af6 100644 --- a/packages/controls/src/widget_selectioncontainer.ts +++ b/packages/controls/src/widget_selectioncontainer.ts @@ -156,7 +156,7 @@ class AccordionView extends DOMWidgetView { // tabs before updating so we don't get spurious changes in the index, // which would then set off another sync cycle. this.updatingChildren = true; - this.pWidget.selection.index = -1; + this.pWidget.selection.index = null; this.children_views.update(this.model.get('children')); this.update_selected_index(); this.updatingChildren = false; From 9b371aac890a84eb011123c12fdda69e5400bf45 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Tue, 11 Jul 2017 07:40:35 +0100 Subject: [PATCH 02/24] Allow none in tab panel --- packages/controls/src/phosphor/tabpanel.ts | 4 ++-- packages/controls/src/widget_selectioncontainer.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/controls/src/phosphor/tabpanel.ts b/packages/controls/src/phosphor/tabpanel.ts index 209d03e462..0cdc3b8cfa 100644 --- a/packages/controls/src/phosphor/tabpanel.ts +++ b/packages/controls/src/phosphor/tabpanel.ts @@ -120,7 +120,7 @@ class TabPanel extends Widget { * Get the index of the currently selected tab. * * #### Notes - * This will be `-1` if no tab is selected. + * This will be `null` if no tab is selected. */ get currentIndex(): number { return this.tabBar.currentIndex; @@ -130,7 +130,7 @@ class TabPanel extends Widget { * Set the index of the currently selected tab. * * #### Notes - * If the index is out of range, it will be set to `-1`. + * If the index is out of range, it will be set to `null`. */ set currentIndex(value: number) { this.tabBar.currentIndex = value; diff --git a/packages/controls/src/widget_selectioncontainer.ts b/packages/controls/src/widget_selectioncontainer.ts index 55f26c5af6..d8e26cec8f 100644 --- a/packages/controls/src/widget_selectioncontainer.ts +++ b/packages/controls/src/widget_selectioncontainer.ts @@ -339,7 +339,7 @@ class TabView extends DOMWidgetView { // tabs before updating so we don't get spurious changes in the index, // which would then set off another sync cycle. this.updatingTabs = true; - this.pWidget.currentIndex = -1; + this.pWidget.currentIndex = null; this.childrenViews.update(this.model.get('children')); this.pWidget.currentIndex = this.model.get('selected_index'); this.updatingTabs = false; From b6ec55626b39672dd9d3da8ed0fdc20d4bf5726a Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Tue, 11 Jul 2017 08:04:44 +0100 Subject: [PATCH 03/24] Add framework for current selection unit tests --- packages/controls/test/src/index.ts | 1 + .../src/phosphor/currentselection_test.ts | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 packages/controls/test/src/phosphor/currentselection_test.ts diff --git a/packages/controls/test/src/index.ts b/packages/controls/test/src/index.ts index 8f17b16a72..dc06b6030e 100644 --- a/packages/controls/test/src/index.ts +++ b/packages/controls/test/src/index.ts @@ -2,3 +2,4 @@ // Distributed under the terms of the Modified BSD License. import './widget_date_test'; +import './phosphor/currentselection_test'; diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts new file mode 100644 index 0000000000..af0db27fb2 --- /dev/null +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -0,0 +1,23 @@ + +import { expect } from 'chai'; + +import { Selection } from '../../../lib/phosphor/currentselection' + +describe('Selection with items', function() { + let selection; + beforeEach(function() { + selection = new Selection(['value-0', 'value-1']) + selection.index = null; + }); + + it('be unselected', function() { + expect(selection.index).to.be.null; + expect(selection.value).to.be.null; + }) + + it('set an item', function() { + selection.index = 1; + expect(selection.index).to.equal(1); + expect(selection.value).to.equal('value-1') + }) +}); From 1f77604fd811eade0f1a8d33e089ac8dcd67afcb Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Wed, 12 Jul 2017 07:42:27 +0100 Subject: [PATCH 04/24] Test signal triggered correctly --- .../test/src/phosphor/currentselection_test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index af0db27fb2..5f0f5678f7 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -1,13 +1,18 @@ import { expect } from 'chai'; +import { spy } from 'sinon'; import { Selection } from '../../../lib/phosphor/currentselection' describe('Selection with items', function() { let selection; + let subscriber; // subscribe to signals from selection + beforeEach(function() { selection = new Selection(['value-0', 'value-1']) selection.index = null; + subscriber = spy() + selection.selectionChanged.connect(subscriber); }); it('be unselected', function() { @@ -20,4 +25,16 @@ describe('Selection with items', function() { expect(selection.index).to.equal(1); expect(selection.value).to.equal('value-1') }) + + it('dispatch a signal when setting an item', function() { + selection.index = 1; + expect(subscriber.calledOnce).to.be.true; + const [_, message] = subscriber.getCall(0).args + expect(message).to.deep.equal({ + previousIndex: null, + previousValue: null, + currentIndex: 1, + currentValue: 'value-1' + }) + }) }); From f481d54fb3dfe7e00926ca0be287b89f34c5eef2 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Wed, 12 Jul 2017 07:50:25 +0100 Subject: [PATCH 05/24] Test for setting by value --- .../test/src/phosphor/currentselection_test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index 5f0f5678f7..43bb5e7592 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -37,4 +37,22 @@ describe('Selection with items', function() { currentValue: 'value-1' }) }) + + it('set a value', function() { + selection.value = 'value-0'; + expect(selection.value).to.equal('value-0'); + expect(selection.index).to.equal(0) + }) + + it('dispatch a signal when setting a value', function() { + selection.value = 'value-0'; + expect(subscriber.calledOnce).to.be.true; + const [_, message] = subscriber.getCall(0).args + expect(message).to.deep.equal({ + previousIndex: null, + previousValue: null, + currentIndex: 0, + currentValue: 'value-0' + }) + }) }); From 52d8e2686711bbf5db6acc53d6b9b38f2f938443 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Wed, 12 Jul 2017 07:55:11 +0100 Subject: [PATCH 06/24] Test for setting index and values to wrong values --- .../test/src/phosphor/currentselection_test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index 43bb5e7592..8716161787 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -55,4 +55,16 @@ describe('Selection with items', function() { currentValue: 'value-0' }) }) + + it('set to null if the index is out of bounds', function() { + selection.index = 22; + expect(selection.index).to.be.null; + expect(selection.value).to.be.null; + }) + + it('set to null if the value is not present', function() { + selection.value = 'not-a-value'; + expect(selection.index).to.be.null; + expect(selection.value).to.be.null; + }) }); From 98e48b6588a0ebdaad7897de4942c2975d7b06ad Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 06:46:29 +0100 Subject: [PATCH 07/24] Unittest for inserting an item in selection --- .../test/src/phosphor/currentselection_test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index 8716161787..9f4982778d 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -7,9 +7,10 @@ import { Selection } from '../../../lib/phosphor/currentselection' describe('Selection with items', function() { let selection; let subscriber; // subscribe to signals from selection + const sequence = ['value-0', 'value-1']; beforeEach(function() { - selection = new Selection(['value-0', 'value-1']) + selection = new Selection(sequence) selection.index = null; subscriber = spy() selection.selectionChanged.connect(subscriber); @@ -67,4 +68,13 @@ describe('Selection with items', function() { expect(selection.index).to.be.null; expect(selection.value).to.be.null; }) + + it('adjust after inserting an item', function() { + const insertedValue = 'value-before-1'; + sequence.splice(1, 0, insertedValue); + // sequence is now [value-0, value-before-1, value-1] + selection.adjustSelectionForInsert(1, insertedValue); + expect(selection.index).to.equal(1); + expect(selection.value).to.equal(insertedValue); + }); }); From 88e4182db2a9fa74d07d5eae1f5c921c7d5b5b57 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 06:49:20 +0100 Subject: [PATCH 08/24] Test for adjusting selection after items moved --- .../test/src/phosphor/currentselection_test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index 9f4982778d..3659b8c136 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -7,9 +7,10 @@ import { Selection } from '../../../lib/phosphor/currentselection' describe('Selection with items', function() { let selection; let subscriber; // subscribe to signals from selection - const sequence = ['value-0', 'value-1']; + let sequence; beforeEach(function() { + sequence = ['value-0', 'value-1'] selection = new Selection(sequence) selection.index = null; subscriber = spy() @@ -77,4 +78,11 @@ describe('Selection with items', function() { expect(selection.index).to.equal(1); expect(selection.value).to.equal(insertedValue); }); + + it('adjust after an item move', function() { + sequence = [sequence[1], sequence[0]] + selection.adjustSelectionForMove(0, 1) + expect(selection.index).to.be.null + expect(selection.value).to.be.null + }); }); From 1a4a0193f77ca6ddf69b24b1d2a72e01ade1f34f Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:05:35 +0100 Subject: [PATCH 09/24] Test for adjusting selection after moving item --- .../controls/test/src/phosphor/currentselection_test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index 3659b8c136..aebfce81a6 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -85,4 +85,11 @@ describe('Selection with items', function() { expect(selection.index).to.be.null expect(selection.value).to.be.null }); + + it('adjust after removing an item', function() { + sequence = [sequence[1]]; + selection.adjustSelectionForRemove(0, 'value-0'); + expect(selection.index).to.be.null; + expect(selection.value).to.be.null; + }); }); From 465c1dd7b71cf7c6bb35640e028fbbd11d4d4b58 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:05:47 +0100 Subject: [PATCH 10/24] Test for clearing selection --- .../controls/test/src/phosphor/currentselection_test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index aebfce81a6..f61f74efab 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -92,4 +92,10 @@ describe('Selection with items', function() { expect(selection.index).to.be.null; expect(selection.value).to.be.null; }); + + it('clear the selection', function() { + selection.clearSelection(); + expect(selection.index).to.be.null; + expect(selection.value).to.be.null; + }); }); From a184d2313ba530454a9884f1a7c480a59dc6dfcd Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:15:19 +0100 Subject: [PATCH 11/24] Tests for changing the item selected --- .../src/phosphor/currentselection_test.ts | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index f61f74efab..a97fa29e56 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -2,8 +2,15 @@ import { expect } from 'chai'; import { spy } from 'sinon'; +import { ArrayExt } from '@phosphor/algorithm'; + import { Selection } from '../../../lib/phosphor/currentselection' +function getLastMessage(subscriber) { + const [_, message] = subscriber.getCall(0).args + return message; +}; + describe('Selection with items', function() { let selection; let subscriber; // subscribe to signals from selection @@ -31,7 +38,7 @@ describe('Selection with items', function() { it('dispatch a signal when setting an item', function() { selection.index = 1; expect(subscriber.calledOnce).to.be.true; - const [_, message] = subscriber.getCall(0).args + const message = getLastMessage(subscriber); expect(message).to.deep.equal({ previousIndex: null, previousValue: null, @@ -99,3 +106,39 @@ describe('Selection with items', function() { expect(selection.value).to.be.null; }); }); + + +describe('Selection with items with an item selected', function() { + let selection; + let subscriber; // subscribe to signals from selection + let sequence; + + beforeEach(function() { + sequence = ['value-0', 'value-1'] + selection = new Selection(sequence) + selection.index = 1; + subscriber = spy() + selection.selectionChanged.connect(subscriber); + }); + + it('set another item', function() { + selection.index = 0; + expect(selection.index).to.equal(0); + expect(selection.value).to.equal('value-0') + const message = getLastMessage(subscriber); + expect(message).to.deep.equal({ + previousIndex: 1, + previousValue: 'value-1', + currentIndex: 0, + currentValue: 'value-0' + }) + }); + + it('adjust after inserting an item', function () { + const insertedValue = 'value-before-1'; + ArrayExt.insert(sequence, 1, insertedValue); + selection.adjustSelectionForInsert(1, insertedValue); + expect(selection.index).to.equal(2); + expect(selection.value).to.equal('value-1'); + }); +}); From 89238bbf2436f14c301c6e1cea1ededbe1c14e03 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:16:21 +0100 Subject: [PATCH 12/24] Use ArrayExt.insert instead of splice. --- packages/controls/test/src/phosphor/currentselection_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index a97fa29e56..b1c81f06d7 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -4,7 +4,7 @@ import { spy } from 'sinon'; import { ArrayExt } from '@phosphor/algorithm'; -import { Selection } from '../../../lib/phosphor/currentselection' +import { Selection } from '../../../lib/phosphor/currentselection'; function getLastMessage(subscriber) { const [_, message] = subscriber.getCall(0).args @@ -79,7 +79,7 @@ describe('Selection with items', function() { it('adjust after inserting an item', function() { const insertedValue = 'value-before-1'; - sequence.splice(1, 0, insertedValue); + ArrayExt.insert(sequence, 1, insertedValue); // sequence is now [value-0, value-before-1, value-1] selection.adjustSelectionForInsert(1, insertedValue); expect(selection.index).to.equal(1); From 8bd2304056cb4cacecedc5535f3c20ac46eb8738 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:22:02 +0100 Subject: [PATCH 13/24] Actually modify items in place --- packages/controls/test/src/phosphor/currentselection_test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index b1c81f06d7..c9bd6fe403 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -87,14 +87,14 @@ describe('Selection with items', function() { }); it('adjust after an item move', function() { - sequence = [sequence[1], sequence[0]] + ArrayExt.reverse(sequence) selection.adjustSelectionForMove(0, 1) expect(selection.index).to.be.null expect(selection.value).to.be.null }); it('adjust after removing an item', function() { - sequence = [sequence[1]]; + ArrayExt.removeAt(sequence, 0) selection.adjustSelectionForRemove(0, 'value-0'); expect(selection.index).to.be.null; expect(selection.value).to.be.null; From e170bb2affbcae8e056d1774093723fdb8e6241b Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:26:17 +0100 Subject: [PATCH 14/24] Tests for removing an item --- .../src/phosphor/currentselection_test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index c9bd6fe403..e10a79a7fc 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -141,4 +141,25 @@ describe('Selection with items with an item selected', function() { expect(selection.index).to.equal(2); expect(selection.value).to.equal('value-1'); }); + + it('adjust after removing an item', function() { + ArrayExt.removeAt(sequence, 0) + selection.adjustSelectionForRemove(0, 'value-0') + expect(selection.index).to.equal(0); + expect(selection.value).to.equal('value-1'); + }); + + it('adjust after removing the selected item', function() { + ArrayExt.removeAt(sequence, 1) + selection.adjustSelectionForRemove(1, 'value-1') + expect(selection.index).to.equal(0); + expect(selection.value).to.equal('value-0'); + const message = getLastMessage(subscriber); + expect(message).to.deep.equal({ + previousIndex: 1, + previousValue: 'value-1', + currentIndex: 0, + currentValue: 'value-0' + }); + }) }); From 04c10b04d61ccc81364e3f262edbc5d439a63584 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:28:02 +0100 Subject: [PATCH 15/24] Long line fix --- ipywidgets/widgets/widget_selectioncontainer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ipywidgets/widgets/widget_selectioncontainer.py b/ipywidgets/widgets/widget_selectioncontainer.py index 404005ef30..a3e9e95c60 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -16,7 +16,9 @@ class _SelectionContainer(Box, CoreWidget): """Base class used to display multiple child widgets.""" _titles = Dict(help="Titles of the pages").tag(sync=True) - selected_index = CInt(help="The index of the selected page.", allow_none=True).tag(sync=True) + selected_index = CInt( + help="The index of the selected page.", allow_none=True + ).tag(sync=True) # Public methods def set_title(self, index, title): From eac8ae448eb02ba130d41c056409762248785838 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:42:54 +0100 Subject: [PATCH 16/24] Validate that selected index is not out of bounds --- .../widgets/tests/test_selectioncontainer.py | 28 +++++++++++++++++++ .../widgets/widget_selectioncontainer.py | 9 +++++- 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 ipywidgets/widgets/tests/test_selectioncontainer.py diff --git a/ipywidgets/widgets/tests/test_selectioncontainer.py b/ipywidgets/widgets/tests/test_selectioncontainer.py new file mode 100644 index 0000000000..9b5dee737e --- /dev/null +++ b/ipywidgets/widgets/tests/test_selectioncontainer.py @@ -0,0 +1,28 @@ +# Copyright (c) Jupyter Development Team. +# Distributed under the terms of the Modified BSD License. + +from unittest import TestCase + +from traitlets import TraitError + +from ipywidgets.widgets import Accordion, HTML + + +class TestAccordion(TestCase): + + def setUp(self): + self.children = [HTML('0'), HTML('1')] + + def test_selected_index_none(self): + accordion = Accordion(self.children, selected_index=None) + state = accordion.get_state() + assert state['selected_index'] is None + + def test_selected_index(self): + accordion = Accordion(self.children, selected_index=1) + state = accordion.get_state() + assert state['selected_index'] == 1 + + def test_selected_index_out_of_bounds(self): + with self.assertRaises(TraitError): + Accordion(self.children, selected_index=-1) diff --git a/ipywidgets/widgets/widget_selectioncontainer.py b/ipywidgets/widgets/widget_selectioncontainer.py index a3e9e95c60..c3e2314a95 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -9,7 +9,7 @@ from .widget_box import Box, register from .widget_core import CoreWidget -from traitlets import Unicode, Dict, CInt +from traitlets import Unicode, Dict, CInt, TraitError, validate from ipython_genutils.py3compat import unicode_type @@ -20,6 +20,13 @@ class _SelectionContainer(Box, CoreWidget): help="The index of the selected page.", allow_none=True ).tag(sync=True) + @validate('selected_index') + def _validated_index(self, proposal): + if proposal.value is None or 0 <= proposal.value < len(self.children): + return proposal.value + else: + raise TraitError('Invalid selection: index out of bounds') + # Public methods def set_title(self, index, title): """Sets the title of a container page. From e5b298c4140426c49fd9b8bfe12032565e76d523 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:45:16 +0100 Subject: [PATCH 17/24] Appropriate file header --- packages/controls/test/src/phosphor/currentselection_test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/controls/test/src/phosphor/currentselection_test.ts b/packages/controls/test/src/phosphor/currentselection_test.ts index e10a79a7fc..1f0fdd130c 100644 --- a/packages/controls/test/src/phosphor/currentselection_test.ts +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -1,3 +1,5 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. import { expect } from 'chai'; import { spy } from 'sinon'; From 59fcdb4f7f31ce6c164b4cfd6e7df6e20c2f6977 Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 08:02:35 +0100 Subject: [PATCH 18/24] Update documentation for Tabs and Accordions --- docs/source/examples/Widget List.ipynb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/examples/Widget List.ipynb b/docs/source/examples/Widget List.ipynb index 81de14e672..ae09854d85 100644 --- a/docs/source/examples/Widget List.ipynb +++ b/docs/source/examples/Widget List.ipynb @@ -1071,7 +1071,7 @@ "\n", "Unlike the rest of the widgets discussed earlier, the container widgets `Accordion` and `Tab` update their `selected_index` attribute when the user changes which accordion or tab is selected. That means that you can both see what the user is doing *and* programmatically set what the user sees by setting the value of `selected_index`.\n", "\n", - "Setting `selected_index = -1` (or any value outside the range of available accordions or tabs) closes all of the accordions or deselects all tabs." + "Setting `selected_index = None` (or any value outside the range of available accordions or tabs) closes all of the accordions or deselects all tabs." ] }, { @@ -1100,7 +1100,7 @@ }, "outputs": [], "source": [ - "accordion.selected_index = -1" + "accordion.selected_index = None" ] }, { From 88ba725882facf963e698171b8d181b58b10436f Mon Sep 17 00:00:00 2001 From: Pascal Bugnion Date: Thu, 13 Jul 2017 07:51:19 +0100 Subject: [PATCH 19/24] Explain selected_index traitlet --- ipywidgets/widgets/widget_selectioncontainer.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ipywidgets/widgets/widget_selectioncontainer.py b/ipywidgets/widgets/widget_selectioncontainer.py index c3e2314a95..03ad770151 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -17,7 +17,11 @@ class _SelectionContainer(Box, CoreWidget): """Base class used to display multiple child widgets.""" _titles = Dict(help="Titles of the pages").tag(sync=True) selected_index = CInt( - help="The index of the selected page.", allow_none=True + help="""The index of the selected page. + + This is either an integer selecting a particular sub-widget, + or None to to have no widgets selected.""", + allow_none=True ).tag(sync=True) @validate('selected_index') From c31ac16581fa65303ee286b9f89fa634edb00de2 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Fri, 14 Jul 2017 11:03:48 -0500 Subject: [PATCH 20/24] fix typo --- ipywidgets/widgets/widget_selectioncontainer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipywidgets/widgets/widget_selectioncontainer.py b/ipywidgets/widgets/widget_selectioncontainer.py index 03ad770151..aa72a081cb 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -20,7 +20,7 @@ class _SelectionContainer(Box, CoreWidget): help="""The index of the selected page. This is either an integer selecting a particular sub-widget, - or None to to have no widgets selected.""", + or None to have no widgets selected.""", allow_none=True ).tag(sync=True) From a2af8b5b47702f3ed71eb48b1f57e84ea8933ef9 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Fri, 14 Jul 2017 11:05:32 -0500 Subject: [PATCH 21/24] We don't allow an index out of range now, so update docs. --- docs/source/examples/Widget List.ipynb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/examples/Widget List.ipynb b/docs/source/examples/Widget List.ipynb index ae09854d85..985d493289 100644 --- a/docs/source/examples/Widget List.ipynb +++ b/docs/source/examples/Widget List.ipynb @@ -1071,7 +1071,7 @@ "\n", "Unlike the rest of the widgets discussed earlier, the container widgets `Accordion` and `Tab` update their `selected_index` attribute when the user changes which accordion or tab is selected. That means that you can both see what the user is doing *and* programmatically set what the user sees by setting the value of `selected_index`.\n", "\n", - "Setting `selected_index = None` (or any value outside the range of available accordions or tabs) closes all of the accordions or deselects all tabs." + "Setting `selected_index = None` closes all of the accordions or deselects all tabs." ] }, { From cc0d4357cf937c72d201e0f32978160d53d272a7 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Fri, 14 Jul 2017 11:08:10 -0500 Subject: [PATCH 22/24] Code style --- packages/controls/src/phosphor/currentselection.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/controls/src/phosphor/currentselection.ts b/packages/controls/src/phosphor/currentselection.ts index ce9c24df60..355b63a3f5 100644 --- a/packages/controls/src/phosphor/currentselection.ts +++ b/packages/controls/src/phosphor/currentselection.ts @@ -126,8 +126,7 @@ class Selection { if (i < 0 || i >= this._array.length) { i = null; } - } - else { + } else { i = null; } From 34dd60a87d11ea88e8798f5ba74215282571f9bb Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Sat, 15 Jul 2017 01:19:37 -0500 Subject: [PATCH 23/24] Translate from phosphor convention of -1 being no selection --- packages/controls/src/phosphor/tabpanel.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/controls/src/phosphor/tabpanel.ts b/packages/controls/src/phosphor/tabpanel.ts index 0cdc3b8cfa..fe9ecf921f 100644 --- a/packages/controls/src/phosphor/tabpanel.ts +++ b/packages/controls/src/phosphor/tabpanel.ts @@ -122,8 +122,10 @@ class TabPanel extends Widget { * #### Notes * This will be `null` if no tab is selected. */ - get currentIndex(): number { - return this.tabBar.currentIndex; + get currentIndex(): number | null { + const currentIndex = this.tabBar.currentIndex; + # Phosphor tab bars have an index of -1 if no tab is selected + return (currentIndex === -1 ? null : currentIndex); } /** @@ -132,8 +134,8 @@ class TabPanel extends Widget { * #### Notes * If the index is out of range, it will be set to `null`. */ - set currentIndex(value: number) { - this.tabBar.currentIndex = value; + set currentIndex(value: number | null) { + this.tabBar.currentIndex = (value === null ? -1 : value); } /** From 95cc187c217dee15daf03a4502c6c808b0b9dd75 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Sat, 15 Jul 2017 01:47:17 -0500 Subject: [PATCH 24/24] Fix js comment --- packages/controls/src/phosphor/tabpanel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controls/src/phosphor/tabpanel.ts b/packages/controls/src/phosphor/tabpanel.ts index fe9ecf921f..d44df5d1f8 100644 --- a/packages/controls/src/phosphor/tabpanel.ts +++ b/packages/controls/src/phosphor/tabpanel.ts @@ -124,7 +124,7 @@ class TabPanel extends Widget { */ get currentIndex(): number | null { const currentIndex = this.tabBar.currentIndex; - # Phosphor tab bars have an index of -1 if no tab is selected + // Phosphor tab bars have an index of -1 if no tab is selected return (currentIndex === -1 ? null : currentIndex); }