kivy / kivy

Open source UI framework written in Python, running on Windows, Linux, macOS, Android and iOS
https://kivy.org
MIT License
17k stars 3.04k forks source link

add more orientation options to BoxLayout #8679

Open gottadiveintopython opened 1 month ago

gottadiveintopython commented 1 month ago

Maintainer merge checklist

This PR adds new orientation options (right-to-left and bottom-to-top) to the BoxLayout.

class BoxLayout:
    orientation = OptionProperty('horizontal', options=('lr', 'rl', 'tb', 'bt',
        'horizontal', 'vertical'))

Potential API break

This might break the API because of the existing code like this:

if boxlayout.orientation == 'horizontal':
    ...
else:
    ...

The else-clause part of the code above always assumes that the boxlayout lays out its children from top to bottom, but after this PR is merged, it can lay them out in any of the following directions: top-to-bottom, bottom-to-top, or right-to-left.

Alternative Idea I was suggested

"Or by introducing a reverse orientation boolean attribute for BoxLayout" -- EBabz at Discord

This might be an option but consider other layouts:

class StackLayout:
    orientation = OptionProperty('lr-tb', options=(
        'lr-tb', 'tb-lr', 'rl-tb', 'tb-rl', 'lr-bt', 'bt-lr', 'rl-bt',
        'bt-rl'))

class GridLayout:
    orientation = OptionProperty('lr-tb', options=(
        'lr-tb', 'tb-lr', 'rl-tb', 'tb-rl', 'lr-bt', 'bt-lr', 'rl-bt',
        'bt-rl'))

Using a boolean property may not be a good choice for maintaining consistency.

DexerBR commented 1 month ago

Hi @gottadiveintopython!

Since version 3.0.0 is an opportunity to "break the API to improve", I think it would be better to just create one property that defines the orientation and direction of adding widgets, what do you think about deprecating the orientation property and use instead:

layout_direction = OptionProperty("left-to-right", options=("left-to-right", "right-to-left", "top-to-bottom", "bottom-to-top"))

Note:

I also wonder if the layout should handle these layout changes independently, since the add_widget method already has the index argument: https://kivy.org/doc/stable/api-kivy.uix.widget.html#kivy.uix.widget.Widget.add_widget

gottadiveintopython commented 1 month ago

@DexerBR

I think it would be better to just create one property that defines the orientation and direction of adding widgets

Is that really a better way? Compared to this PR, it seems to just make the API inconsistent as StackLayout and GridLayout don't have that property.

I also wonder if the layout should handle these layout changes independently, since the add_widget method already has the index argument: https://kivy.org/doc/stable/api-kivy.uix.widget.html#kivy.uix.widget.Widget.add_widget

I believe so because the index (i.e. the order of the children property) isn't just for layout; it also affects the drawing order and the order of receiving touch events. Having the ability to change the layout without affecting these two orders would certainly come in handy somewhere.

nanouasyn commented 4 weeks ago

I believe so because the index (i.e. the order of the children property) isn't just for layout; it also affects the drawing order and the order of receiving touch events. Having the ability to change the layout without affecting these two orders would certainly come in handy somewhere.

The fact that the ordinal number of the widget in the children list (which determines the sequence of rendering and receiving touch events) is also necessarily the ordinal number of the widget in the sequence of placing widgets inside the BoxLayout is really strange. Why do the widgets at the end of the list have to be in the foreground relative to the previous widgets? It doesn't make sense. It looks like a random solution, an implementation detail. But your decision does not change the situation in any way. Just as there is no point in, for example, lower-placed widgets always overlapping widgets at the top, so there is no point in higher-placed widgets always overlapping widgets at the bottom. We're just changing one weird solution to another weird solution.

In any case, it would be correct to consider any situation where the widgets inside the BoxLayout overlap each other as a mistake. The solution to the problem would be to provide a mechanism for arbitrary manipulation of the placement order, independent of the order of rendering and processing touches. For example, the layout_index property, which any widget (or some kind of BoxLayoutItem wrapper) can set to change its placement order. Or the foreground_widgets property for BoxLayout, which allows you to specify the widgets that will be in the foreground in the specified order, regardless of the placement order.

nanouasyn commented 4 weeks ago

What about accepting @DexerBR's suggestion, but not deprecating orientation? For example, add the direction property. Whenever orientation is set to 'horizontal', set direction to 'lr'. Whenever orientation is set to 'vertical', set direction to 'tb'. Whenever direction is set to 'lr' or 'rl', set orientation to 'horizontal'. Whenever direction is set to 'tb' or 'bt', set orientation to 'vertical'. Then users will be able to choose which API to use. The alternative is that the orientation property accepts values only from the set 'lr', 'rl', 'tb', 'bt'. That is, to abandon the 'horizontal' and 'vertical', and break the API completely, so as not to confuse users.

gottadiveintopython commented 3 weeks ago

@nanouasyn

If I understand Kivy correctly, the rendering sequence needs to be opposite to the sequence of receiving touch events in order for its GUI system to work so

the widgets at the end of the list have to be in the foreground relative to the previous widgets

itself should be fine. However, yes, the layout doesn't have to depend on the children sequence. It's possible to introduce the layout_index you suggested, or even better, use the pos_hint:

BoxLayout:
    Widget:
        pos_hint: {'layout_index': 0, }
    Widget:
        pos_hint: {'layout_index': 1, }

But I think this excessively breaks the API even though Kivy is moving towards the next major version, and more importantly, it doesn't sound convenient. What if you need to add a widget to a BoxLayout where the layout_index values of its children are unknown? You'll have to gather all those values and calculate a suitable position for the widget you want to add.

@DexerBR

I also wonder if the layout should handle these layout changes independently, since the add_widget method already has the index argument: https://kivy.org/doc/stable/api-kivy.uix.widget.html#kivy.uix.widget.Widget.add_widget

I don't think I fully addressed this in the previous comment. You meant you can add a widget on the left/top side of the existing children by using the index like this, right?

boxlayout.add_widget(child, index=len(boxlayout.children))

Still, it feels strange to me that horizontal refers to left-to-right, which is the positive direction of the x-axis, while vertical refers to top-to-bottom, which is the negative direction of the y-axis. Isn't it better to support all four directions to fix this inconsistent design?

@DexerBR @nanouasyn

I'm starting to think @DexerBR's idea might be just as good as mine because it reduces the risk of breaking the API.

if boxlayout.orientation == 'horizontal':
    # assumes the orientation to be horizontal
    # assumes the direction to be left-to-right
else:
    # assumes the orientation to be vertical
    # assumes the direction to be top-to-bottom

Consider existing code like the above. @DexerBR's approach might disrupt the assumption regarding the direction, but it doesn't affect the assumption regarding the orientation, whereas mine could disrupt both.

In summary, @DexerBR's and EBabz's prioritize the compatibility, while mine prioritizes the consistency in the API. I think both are equally good.

akshayaurora commented 3 weeks ago

I would prefer not adding this feature to BoxLayout.

For a specific way to layout your widget it would make a lot more sense to have a LayoutOrderBehavior

<OrderdBoxLayout@BoxLayout+LayoutOrderBehavior>
    layout_order: 'tb-lr'

This would be scalable in that one can then in that case combine this behaviour with other layouts... Think users having their own custom layouts, without having to specifically use BoxLayout.

gottadiveintopython commented 2 weeks ago

@akshayaurora

Why do you prefer BoxLayout to have such a different design compared to the others (GridLayout, StackLayout)? or did you mean modify the others as well?

And I don't think the LayoutOrderBehavior is a realistic idea because most of the computations needed for layouts are tied to each individual layout, and cannot be reused across others. If it were introduced, it would look like this:

class LayoutOrderBehavior:
    def very_little_computation_that_can_be_used_across_many_layouts():
        ...
    def boxlayout_specific_computation():
        ...
    def gridlayout_specific_computation():
        ...
    def stacklayout_specific_computation():
        ...
akshayaurora commented 2 weeks ago

GridLayout, StackLayout offer you the same option, i.e to choose the order of the widgets to be added.

You can even just override the existing behaviour using your own LayoutBehavior locally, for instance.

Current GridLayout does not layout the widgets with a exactly equal distance from each other. So locally I use

<SGridLayout@EquiLayoutBehavior+GridLayout>
    padding: dp(9), dp(9)

Where the class EquiLayoutBehavior is something like this.

class EquiLayoutBehavior(EventDispatcher):

    Builder.load_string('''
<EquiLayoutBehavior>
    padding: 0, 0
    cols: max(1, int((self.width - (self.padding[0])*2)/(self.children[-1].width))) if self.children else 1
''')

    def do_layout(self, *largs):
        if not self.children:
            return

        idx = 0
        children = self.children[::-1]
        x = self.x
        top = self.top - self.padding[1]
        padding = self.padding
        width = self.width - (self.padding[0]*2)
        cols = self.cols
        child = None
        right = self.right - padding[0]
        r3 = right - children[1].width
        try:
            while idx <= len(children):
                x =  self.x + padding[0]

                for z in range(cols):
                    child = children[idx]
                    child.x = x
                    x += (width - (right - r3))/max(1, (cols-1))
                    child.top = top
                    idx += 1

                # next column
                top -= child.height + self.spacing[1]
        except IndexError:
            # print(idx)
            pass
        self.minimum_height =  (self.top - child.y) + (padding[1]*4) + (self.spacing[1]*4)

There could be many different ways a developer could choose to display/layout their widgets.

You should not be changing the existing layout but extending it in your own code based on your own needs.

Changing it for every one would break it for every one who relies on that behaviour.

Adding a Behaviour like this would be preferable.