Change Layout.border from a property to a trait

[fleming79]

In v7 Layout.border was a trait and meant that the border could be linked between widgets. In v8 it was changed to a property which non longer supports this functionality. To achieve the same functionality all 4 borders (top, left, bottom, right) would need to be linked, but is somewhat clunky.

The changes suggested haven't been tested, but approximate the functionality that would be useful. Making this change might help reduce the pain of switching from v7 to v8.

[github-actions[bot]]

:point_left: Launch a binder notebook on branch fleming79/ipywidgets-revert-layout.border/master

[jasongrout]

I like the idea you propose about having a non-synced trait so that link can still work, you can still observe value changes, etc. This is a bit tricky since it doesn't make sense to have a single border value if the separate border_* traits are different.

I see you preserve the logic of setting the border sets all the border_* underlying traits. Can we also add an observer on the border_* traits, so that when they are set, they will set the border to None if they are then out of sync? There may be some subtle issues about who is updating what with this circular logic.

[fleming79]

Thanks for having a look and the suggested change. Unfortunately I can't think of a simple way to set border to None by observing a border_* which would give a reliable behavior (If you consider linking and observing). My preference would be to leave it 'one way' to keep the logic as simple as possible. With the current logic you can set all borders at once with border and later customize a border_* afterwards.

If the change is approved, updating the documentation about border and border_* could be useful? I also found a note about backwards compatibility of Layout.border in the migration guide.

[jasongrout]

My preference would be to leave it 'one way' to keep the logic as simple as possible. With the current logic you can set all borders at once with border and later customize a border_* afterwards.

We debated these sorts of options when we made the change for border_* attributes, and decided that it would be more confusing if border was inconsistent with the underlying border_* attributes. I can see that approach leading to a lot of future issues/complaints since the behavior is surprising/inconsistent for many users.

Is the problem you are referring to the issue of a circular logic of setting border then sets the border_* attributes, which then can trigger a border set? Sometimes we take care of that by setting a flag when we set border_* attributes from within the border attribute. Here's an (untested) stab at it:

    __broadcasting_border__ = False

    @observe("border_left", "border_right", "border_top", "border_bottom")
    def _broadcast_border(self, proposal):
        if self.__broadcasting_border__:
            return

        try:
            self.__broadcasting_border__ = True
            b = self.border_left
            if b == self.border_right and b == self.border_top and b == self.border_bottom:
                self.border = b
            else:
                self.border = None
        finally:
            self.__broadcasting_border__ = False

    @observe("border")
    def _observe_border(self, change):
        if self.__broadcasting_border__:
            return

        try:
            self.__broadcasting_border__ = True
            with self.hold_trait_notifications():
                for side in ['top', 'right', 'bottom', 'left']:
                    setattr(self, "border_" + side, change.new)
        finally:
            self.__broadcasting_border__ = False

Taking a step back, an alternative is the more awkward linking, like you point out:

for side in ('left', 'right', 'bottom', 'top'):
    link((x.layout, 'border_'+side), (y.layout, 'border_'+side))

(as a bonus, jslink then works too)

Note that if you are synchronizing all layout attributes, you can use the exact same layout for each widget.

[fleming79]

I was actually thinking about breaking links applied by the user when I wrote that. In your suggestion you've use hold_trait_notifications which I guess should prevent a broken link error from occurring.

I agree this is a tricky issue with no perfect solution.

[jasongrout]

Another point brought up by @vidartf in the ipywidgets dev meeting on Tuesday when we were discussing this:

If a user links two borders, then changes the border_top of the first widget, then the second widget will suddenly have no border, i.e., it won't replicate the border_top of the first widget. This is probably surprising to the user. If instead the user links each of the 4 border attributes separately, the two widget borders will remain in sync.

In pseudocode (that hasn't been run, so please excuse typos):

import ipywidgets as w
slider = w.IntSlider()
text = w.Text()
w.link((slider.layout, 'border'), (text.layout, 'border'))
slider.layout.border = '1px solid red'
display(w.HBox([slider, text]))

Now this is probably surprising what happens to the text widget

slider.layout.border_bottom = '3px solid green'

While this would work as expected - keeping the borders looking the same:

import ipywidgets as w
slider = w.IntSlider()
text = w.Text()
for side in ('top', 'left', 'right', 'bottom'):
    w.link((slider.layout, 'border_'+side), (text.layout, 'border_'+side))
slider.layout.border = '1px solid red'
display(w.HBox([slider, text]))

slider.layout.border_bottom = '3px solid green'