Fix selection container default index

[SylvainCorlay]

cf discussions on #1822

• It does not make sense to default to 0 for an empty list of children.
• The default value of most attributes allowing None is None.

[jasongrout]

I think it makes sense for Tab to default to the first item selected. Notice the logic in the selection widgets about choosing a default selection:

ipywidgets/ipywidgets/widgets/widget_selection.py

Line 209 in 4519ca8

self.index = 0 if len(options) > 0 else None

We could do something like that.

I agree that a default of None for the accordion makes sense.

[SylvainCorlay]

OK, I changed the logic to the following:

• general default for selection container index is None.
• general behavior when changing the children is setting the new selected index to None when the previous value was higher than the new number of children.
• in the case of the tab:
  • if we provide a non-empty children list, the first tab will be selected by default.

[SylvainCorlay]

@jasongrout are you ok with this new logic? If yes, I will merge the companion PR in xwidgets.

[jasongrout]

This means that if I keep adding a tab to the end (I think @rmenegaux did this, for example), the selected index keeps being reset to 0.

[SylvainCorlay]

That is right. Although, we don't have operational transforms on sequences, we change all at once.

Since setting the selected index to None is not something we want for tabs, this is one way to go.

[jasongrout]

This doesn't necessarily select the same child (if we delete a child from the front, now the selected container is not the same), which may be confusing.

[SylvainCorlay]

same as above

[jasongrout]

I'm thinking through various scenarios with the current behavior and your new behavior. Two scenarios are:

1. Creating a selection container, then deleting the first container. What should be shown after that?

2. Creating a selection container and adding a new container at the end. What should be shown after that?

[jasongrout]

Part of the problem here is that we don't have the notion of inserting or removing a single container - we only have the notion of refreshing the entire children list. When you have more semantic information about what was done (e.g., deleting a tab), you can do something more intelligent like selecting the tab next to it. See, for example, the phosphor insert and remove behavior options at http://phosphorjs.github.io/phosphor/api/widgets/classes/tabbar.html#insertbehavior.

[SylvainCorlay]

This new behavior is still better than the current one, which does not handle the case of an empty list (and causes failures in the C++ backend).

We could try to check if old_children[selected_index] is an item in the new sequence, but this will also be the case when multiple tabs have the same content.

[jasongrout]

Is there a way to preserve the current behavior for cases where the children list is nonempty, and only insert a workaround for when the children list is empty?

If we are changing the current behavior for non-empty children lists, we're breaking user's code, so we really should wait until 8.0. I can imagine the frustration of updating to a patch or even a minor release and having your code managing the children of a selection container break.

[SylvainCorlay]

We should make a decision on this one for 8.0 IMO.

I am not so opinionated about the solution but I think that selection containers should be default-constructible.

[jasongrout]

Rebased on master

[jasongrout]

I thought more about it, and I think it should be the user's responsibility to set the selected index to something that makes sense when they change the children list, especially for the tab widget:

1. Because we have no semantic information about what the user did to the children list, it's impossible to guess what the right behavior should be. Encoding a guess may make it more difficult for the user to implement their desired behavior
2. For tab widgets, we do actually support selecting no tab
3. We'll have some very minor logic to not touch the selected index if it still makes sense (i.e., if the children array is at least that big). If it doesn't make sense, we just set it to None and rely on the user to set the index to something that makes sense.

[jasongrout]

Thanks @SylvainCorlay!