diff --git a/docs/source/examples/Widget List.ipynb b/docs/source/examples/Widget List.ipynb index 81de14e672..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 = -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` closes all of the accordions or deselects all tabs." ] }, { @@ -1100,7 +1100,7 @@ }, "outputs": [], "source": [ - "accordion.selected_index = -1" + "accordion.selected_index = None" ] }, { 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 e536cffe9d..aa72a081cb 100644 --- a/ipywidgets/widgets/widget_selectioncontainer.py +++ b/ipywidgets/widgets/widget_selectioncontainer.py @@ -9,14 +9,27 @@ 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 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. + + This is either an integer selecting a particular sub-widget, + or None to have no widgets selected.""", + 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): diff --git a/packages/controls/src/phosphor/currentselection.ts b/packages/controls/src/phosphor/currentselection.ts index 0cf9af0301..355b63a3f5 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,19 @@ 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 +199,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 +245,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 +288,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 +339,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 +353,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/phosphor/tabpanel.ts b/packages/controls/src/phosphor/tabpanel.ts index 209d03e462..d44df5d1f8 100644 --- a/packages/controls/src/phosphor/tabpanel.ts +++ b/packages/controls/src/phosphor/tabpanel.ts @@ -120,20 +120,22 @@ 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; + 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); } /** * 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; + set currentIndex(value: number | null) { + this.tabBar.currentIndex = (value === null ? -1 : value); } /** diff --git a/packages/controls/src/widget_selectioncontainer.ts b/packages/controls/src/widget_selectioncontainer.ts index 54e84ba6ca..d8e26cec8f 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; @@ -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; 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..1f0fdd130c --- /dev/null +++ b/packages/controls/test/src/phosphor/currentselection_test.ts @@ -0,0 +1,167 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +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 + let sequence; + + beforeEach(function() { + sequence = ['value-0', 'value-1'] + selection = new Selection(sequence) + selection.index = null; + subscriber = spy() + selection.selectionChanged.connect(subscriber); + }); + + 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') + }) + + it('dispatch a signal when setting an item', function() { + selection.index = 1; + expect(subscriber.calledOnce).to.be.true; + const message = getLastMessage(subscriber); + expect(message).to.deep.equal({ + previousIndex: null, + previousValue: null, + currentIndex: 1, + 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' + }) + }) + + 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; + }) + + it('adjust after inserting an item', function() { + const insertedValue = 'value-before-1'; + 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); + expect(selection.value).to.equal(insertedValue); + }); + + it('adjust after an item move', function() { + 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() { + ArrayExt.removeAt(sequence, 0) + selection.adjustSelectionForRemove(0, 'value-0'); + 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; + }); +}); + + +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'); + }); + + 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' + }); + }) +});