New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow `selected_index=none` in selection container widgets #1495

Merged
merged 24 commits into from Jul 15, 2017

Conversation

Projects
None yet
2 participants
@pbugnion
Member

pbugnion commented Jul 11, 2017

Prior to this commit, the way to unset the selection for a tab or accordion widget was to set selected_index = -1. Having to set it to None would be better (see issue #1493 ).

This PR allows setting the index to None. The bulk of the changes are in the Selection class, which now allows setting the index to null.

This is what's left to do:

  • explicitly disable setting selected_index to an out of range value
  • unit tests for Selection class
  • amend docs

This is now ready for review.

@pbugnion

This comment has been minimized.

Member

pbugnion commented Jul 11, 2017

screen shot 2017-07-11 at 07 30 17

@pbugnion pbugnion force-pushed the pbugnion:selection-container-allow-none branch from f3806bc to b6ec556 Jul 11, 2017

@jasongrout jasongrout added this to the 7.0 milestone Jul 11, 2017

@pbugnion pbugnion changed the title from [WIP] Allow `selected_index=none` in selection container widgets to Allow `selected_index=none` in selection container widgets Jul 13, 2017

@pbugnion pbugnion force-pushed the pbugnion:selection-container-allow-none branch from 7494306 to 88ba725 Jul 13, 2017

pbugnion and others added some commits Jul 13, 2017

@@ -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`.

This comment has been minimized.

@jasongrout

jasongrout Jul 14, 2017

Member

It looks like the logic here is still missing for out of range values.

This comment has been minimized.

@jasongrout

jasongrout Jul 14, 2017

Member

Actually, you don't need to do anything here - the tab bar handles the logic to set its current index to -1 if out of range. We'll just handle the translation to null up where we retrieve the index.

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

This comment has been minimized.

@jasongrout

jasongrout Jul 14, 2017

Member

Logic needs to be added here to return null if the tab bar index is -1.

@jasongrout jasongrout merged commit 05dd4e4 into jupyter-widgets:master Jul 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jasongrout

This comment has been minimized.

Member

jasongrout commented Jul 15, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment