Merge pull request #1495 from pbugnion/selection-container-allow-none

Allow `selected_index=none` in selection container widgets

Author: jasongrout

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"
    ]
   },
   {

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)

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):

packages/controls/src/phosphor/currentselection.ts

@@ -93,7 +93,7 @@ class Selection<T> {
    */
   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<T> {
    * 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<T> {
    * @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<T> {
 
     // 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<T> {
     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<T> {
     // 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<T> {
     }
 
     // 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<T> {
    */
   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<T> = null;

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);
   }
 
   /**

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;

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';