GridLayout: Add 'orientation' property to GridLayout #6741

gottadiveintopython · 2020-02-19T07:58:05Z

Continuation of #6593 .

~~RecycleGridLayout._update_rows_cols_sizes() is not tested at all.~~
This is what I'm using on the test. (last update 2020 May 31)

gottadiveintopython · 2020-02-23T04:52:08Z

kivy/uix/recyclegridlayout.py

-        row = len(self._cols)
-        if row:
+        stride = len(self._cols) if self._fills_row_first else len(self._rows)
+        if stride:


Not sure why this truthy check is necessary. Both len(self._cols) and len(self._rows) look always greater than zero because of:

kivy/kivy/uix/gridlayout.py

Lines 303 to 304 in 53290db

current_cols = max(1, current_cols)

current_rows = max(1, current_rows)

tshirtman

LGTM, especially the tests, sorry for taking so much time to review, it's a really useful contribution, and the code looks good. It's great that we have it for RecycleGridLayout as well, that was certainly not trivial.
I fear the way to compute the indices might be a bit heavy if we have a lot of items in the recycleview, as it'll have to create a really big list that maybe we could do without if we kept various divmod cases, but it might not be such a big deal, compared to the memory requirements of such an usage anyway, so i think it's fine to merge as is, and optimize later if we need and can.

gottadiveintopython · 2020-09-30T11:58:31Z

Yes, there are couple of places that I should have avoided list/tuple creation.

gottadiveintopython · 2020-10-02T04:20:13Z

kivy/uix/recyclegridlayout.py

@@ -39,16 +39,17 @@ def _fill_rows_cols_sizes(self):
        cols_sh_min, rows_sh_min = self._cols_sh_min, self._rows_sh_min
        cols_sh_max, rows_sh_max = self._cols_sh_max, self._rows_sh_max
        self._cols_count = cols_count = [defaultdict(int) for _ in cols]
+        # !! bottom-to-top, the opposite of the other attributes.


This might be a misunderstanding

matham · 2020-10-02T06:41:29Z

kivy/uix/gridlayout.py

        if current_cols is None:
+            if self._fills_row_first:
+                Logger.warning(


I'm just taking a quick skim, and that should be a error, not a warning. We should almost never have warnings in code. This is an error in the configuration so a error should be raised.

ok, got it.

gottadiveintopython · 2020-10-02T10:34:18Z

https://kivy.org/doc/master/api-kivy.uix.bubble.html#kivy.uix.bubble.Bubble.orientation

Bubble is a subclass of GridLayout and already has orientationproperty, which may be the cause of errors on unit tests. I just noticed after I made GridLayout raise an exception when the configuration is wrong. (@matham 's suggestion above).

~~You can confirm the errors by using this branch~~

I should remove orientation from Bubble, right?

tshirtman · 2020-10-02T10:43:04Z

Err, that would be an api break, i'm not sure how many people use that feature, i guess we could make "vertical" and horizontal" each aliases of one of the existing option in GridLayout? (to be clear, that would be a change in GridLayout, and we would remove the property in Bubble).

gottadiveintopython · 2020-10-02T11:02:09Z

That still breaks the API if there are code like this:

if bubble.orientation != 'horizontal':
    # assuming the orientation is 'vertical', but there are 'lr-tb', 'rl-tb' etc...

So I believe we need another name if we don't want to break existing code. I prefer removing it from Bubble though.

(Edit) @tshirtman But yours may be the safest option. I'll do it in that way if no one argue with that.

tshirtman · 2020-10-02T11:27:43Z

Ah, i just read more in detail, and it doesn't do what i had assumed, it sets the cols/rows for the nested gridlayout (it's a grid layout with another grid layout inside it for content…) so just relying on GridLayout's implementation wouldn't work.

Replacing an OptionProperty with another with different options used to work though? why does the change in GridLayout affects the Bubble?

gottadiveintopython · 2020-10-02T12:06:42Z

ok I'll try in that way.

…tion is wrong

* avoid temporal list/tuple creation * rename internal function * avoid holding pre-calculated indices because it might be a problem when a RecycleView has a lot of items * edit comment * make '_fills_row_first' into public api * make some non-member functions into a member function in order to avoid code duplication * keep the compatibility with Bubble as possible * (unit tests fail) raise an exception instead of warning when the configuration is wrong * (unit tests fail) revert 'orientation' * modify TabbedPanel due to the 'orientation' * small improvement * revert unnecessary changes * Revert "(unit tests fail) raise an exception instead of warning when the configuration is wrong" This reverts commit 941f933. * make 'fills_xxx'properties into private * make GridLayout behave the same as before #6741 when the configration is wrong * revert unnecessary changes * modify '_fills_xxx' properties * remove unnecessary code * revert unnecessary changes

Pre kivy 2.0, "orientation" was undefined for GridLayouts, not sure why we were setting it, it was a no-op. kivy 2.0 added meaning to the field, and the values we were setting it to are invalid. related: kivy/kivy#6741 kivy/kivy#7142 ----- traceback: E | gui.kivy.uix.dialogs.crash_reporter.ExceptionHook | exception caught by crash reporter Traceback (most recent call last): File "/home/user/.local/lib/python3.8/site-packages/kivy/lang/builder.py", line 705, in _apply_rule setattr(widget_set, key, value) File "kivy/weakproxy.pyx", line 35, in kivy.weakproxy.WeakProxy.__setattr__ File "kivy/properties.pyx", line 498, in kivy.properties.Property.__set__ File "kivy/properties.pyx", line 542, in kivy.properties.Property.set File "kivy/properties.pyx", line 533, in kivy.properties.Property.set File "kivy/properties.pyx", line 1253, in kivy.properties.OptionProperty.check ValueError: GridLayout.orientation is set to an invalid option 'vertical'. Must be one of: ['lr-tb', 'tb-lr', 'rl-tb', 'tb-rl', 'lr-bt', 'bt-lr', 'rl-bt', 'bt-rl'] During handling of the above exception, another exception occurred: Traceback (most recent call last): File "kivy/_clock.pyx", line 645, in kivy._clock.CyClockBase._process_events File "kivy/_clock.pyx", line 218, in kivy._clock.ClockEvent.tick File "/home/user/wspace/electrum/electrum/gui/kivy/uix/ui_screens/receive.kv", line 141, in <lambda> on_release: Clock.schedule_once(lambda dt: s.expiration_dialog(s)) File "/home/user/wspace/electrum/electrum/gui/kivy/uix/screens.py", line 517, in expiration_dialog d = ChoiceDialog(_('Expiration date'), pr_expiration_values, self.expiry(), callback) File "/home/user/wspace/electrum/electrum/gui/kivy/uix/dialogs/choice_dialog.py", line 53, in __init__ Factory.Popup.__init__(self) File "/home/user/.local/lib/python3.8/site-packages/kivy/uix/modalview.py", line 187, in __init__ super(ModalView, self).__init__(**kwargs) File "/home/user/.local/lib/python3.8/site-packages/kivy/uix/anchorlayout.py", line 68, in __init__ super(AnchorLayout, self).__init__(**kwargs) File "/home/user/.local/lib/python3.8/site-packages/kivy/uix/layout.py", line 76, in __init__ super(Layout, self).__init__(**kwargs) File "/home/user/.local/lib/python3.8/site-packages/kivy/uix/widget.py", line 359, in __init__ self.apply_class_lang_rules( File "/home/user/.local/lib/python3.8/site-packages/kivy/uix/widget.py", line 463, in apply_class_lang_rules Builder.apply( File "/home/user/.local/lib/python3.8/site-packages/kivy/lang/builder.py", line 541, in apply self._apply_rule( File "/home/user/.local/lib/python3.8/site-packages/kivy/lang/builder.py", line 710, in _apply_rule raise BuilderException(rule.ctx, rule.line, kivy.lang.builder.BuilderException: Parser: File "<inline>", line 21: ... 19: GridLayout: 20: row_default_height: '48dp' >> 21: orientation: 'vertical' 22: id: choices 23: cols: 2 ... ValueError: GridLayout.orientation is set to an invalid option 'vertical'. Must be one of: ['lr-tb', 'tb-lr', 'rl-tb', 'tb-rl', 'lr-bt', 'bt-lr', 'rl-bt', 'bt-rl'] File "/home/user/.local/lib/python3.8/site-packages/kivy/lang/builder.py", line 705, in _apply_rule setattr(widget_set, key, value) File "kivy/weakproxy.pyx", line 35, in kivy.weakproxy.WeakProxy.__setattr__ File "kivy/properties.pyx", line 498, in kivy.properties.Property.__set__ File "kivy/properties.pyx", line 542, in kivy.properties.Property.set File "kivy/properties.pyx", line 533, in kivy.properties.Property.set File "kivy/properties.pyx", line 1253, in kivy.properties.OptionProperty.check

gottadiveintopython added the Component: Widgets kivy/uix, style.kv label Feb 19, 2020

gottadiveintopython changed the title ~~add 'orientation'property to GridLayout~~ [WIP] add 'orientation'property to GridLayout Feb 19, 2020

gottadiveintopython force-pushed the add_orientation_to_GridLayout branch from 6a39568 to 039656e Compare February 23, 2020 04:32

gottadiveintopython commented Feb 23, 2020

View reviewed changes

gottadiveintopython assigned gottadiveintopython and unassigned gottadiveintopython Feb 23, 2020

sairam4123 previously approved these changes Mar 13, 2020

View reviewed changes

gottadiveintopython dismissed sairam4123’s stale review via e47aae7 March 15, 2020 04:47

gottadiveintopython force-pushed the add_orientation_to_GridLayout branch from e47aae7 to de52059 Compare May 27, 2020 14:23

gottadiveintopython added 2 commits May 28, 2020 09:28

add 'orientation'property to GridLayout

0607a90

modify the recycle one as well

3380346

gottadiveintopython force-pushed the add_orientation_to_GridLayout branch from de52059 to 3380346 Compare May 28, 2020 00:53

gottadiveintopython changed the title ~~[WIP] add 'orientation'property to GridLayout~~ add 'orientation'property to GridLayout May 28, 2020

tshirtman approved these changes Sep 27, 2020

View reviewed changes

tshirtman merged commit e7f2325 into kivy:master Sep 27, 2020

gottadiveintopython mentioned this pull request Oct 1, 2020

GridLayout : optimize GridLayout #7118

Merged

gottadiveintopython deleted the add_orientation_to_GridLayout branch October 1, 2020 14:11

gottadiveintopython commented Oct 2, 2020

View reviewed changes

matham reviewed Oct 2, 2020

View reviewed changes

gottadiveintopython added a commit to gottadiveintopython/kivy that referenced this pull request Oct 10, 2020

make GridLayout behave the same as before kivy#6741 when the configra…

292932c

…tion is wrong

matham added this to the 2.0.0 milestone Oct 28, 2020

matham changed the title ~~add 'orientation'property to GridLayout~~ GridLayout: Add 'orientation' property to GridLayout Dec 8, 2020

gottadiveintopython mentioned this pull request Dec 24, 2020

TextInput: cut-cope-paste bubble doesn't appear when using selection handles #7247

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GridLayout: Add 'orientation' property to GridLayout #6741

GridLayout: Add 'orientation' property to GridLayout #6741

gottadiveintopython commented Feb 19, 2020 •

edited

Loading

gottadiveintopython Feb 23, 2020

tshirtman left a comment

gottadiveintopython commented Sep 30, 2020

gottadiveintopython Oct 2, 2020

matham Oct 2, 2020

gottadiveintopython Oct 2, 2020

gottadiveintopython commented Oct 2, 2020 •

edited

Loading

tshirtman commented Oct 2, 2020 •

edited

Loading

gottadiveintopython commented Oct 2, 2020 •

edited

Loading

tshirtman commented Oct 2, 2020

gottadiveintopython commented Oct 2, 2020

	current_cols = max(1, current_cols)
	current_rows = max(1, current_rows)

GridLayout: Add 'orientation' property to GridLayout #6741

GridLayout: Add 'orientation' property to GridLayout #6741

Conversation

gottadiveintopython commented Feb 19, 2020 • edited Loading

gottadiveintopython Feb 23, 2020

Choose a reason for hiding this comment

tshirtman left a comment

Choose a reason for hiding this comment

gottadiveintopython commented Sep 30, 2020

gottadiveintopython Oct 2, 2020

Choose a reason for hiding this comment

matham Oct 2, 2020

Choose a reason for hiding this comment

gottadiveintopython Oct 2, 2020

Choose a reason for hiding this comment

gottadiveintopython commented Oct 2, 2020 • edited Loading

tshirtman commented Oct 2, 2020 • edited Loading

gottadiveintopython commented Oct 2, 2020 • edited Loading

tshirtman commented Oct 2, 2020

gottadiveintopython commented Oct 2, 2020

gottadiveintopython commented Feb 19, 2020 •

edited

Loading

gottadiveintopython commented Oct 2, 2020 •

edited

Loading

tshirtman commented Oct 2, 2020 •

edited

Loading

gottadiveintopython commented Oct 2, 2020 •

edited

Loading