beeware
Fix visibility and display support in Pack()
This is a first attempt to resolve two issues within the Pack() implementation.
- On GTK, the visibility=HIDDEN is not working
- On all platform except Web, display=NONE is not working
The the visibility problem on GTK. It seams to be related to the fact we are making call to show_all() which effectively make all children of the container visible. There are a couple of way to get this fixed. I decide to manually call show() on every widget to fine tune when to show or hide it.
To match the behavior of other platform and reserve enough space for the widget in the interface, when getting the preferred size, we need to make the widget visible. Otherwise, the return values are zero (0).
For display=NONE, the problem is mush simpler, I simply skip all the calculation in Pack() for every children with display==NONE. This exclude them from the layout.
- Fixes #2447
PR Checklist:
- [X] All new features have been tested : On GTK & Windows only.
- [ ] All new features have been documented : Already documented.
- [X] I have read the CONTRIBUTING.md file
- [X] I will abide by the code of conduct
@freakboy3742 Thanks for your review. Honestly, I will need some help to complete this change.
(1) I've added a note to the change logs.
(2) Regarding the addition or fixing the current test, I have no clue where to start. But I would like to agree about the implementation before writing some test to avoid any rework.
(3) I've read the GTK documentation and I can't find another solution. If the widget is hidden, we need to make it visible in order to measure it. I changed the implementation to avoid toggling the visibility if the widget is visible to avoid extra calls. Visually, I don't see any flickering with current implementations. But I'm not fluent enough in GTK to know if another approach exists.
@freakboy3742 Thanks for your review. Honestly, I will need some help to complete this change.
No problems - that's what we're here for.
(2) Regarding the addition or fixing the current test, I have no clue where to start. But I would like to agree about the implementation before writing some test to avoid any rework.
Honestly - regarding ordering: this is backwards. The correct behavior is completely independent of the implementation, and in an ideal world you always build the tests of correct behavior before you have the implementation, because that way the test is independent of the implementation. Having tests before implementing is preferable, because you start with tests that fail, and you keep implementing until the tests pass :-)
As for how to write them: Toga has two sets of tests.
The first is the core tests; these are a set of vanilla pytests, running against a "dummy" backend that doesn't have any visual manifestation. This is where the literal math of the Pack algorithm is validated - the core/tests/style/pack folder contains a suite of tests that exercise all the various options and combinations of options, and how that influences the calculation of position and size of boxes being laid out. We clearly don't have any tests that exercise that case of a display=NONE style causing a widget to be omitted from layout.
The second is the testbed tests. This is the "live" test against actual backends, validating that the GTK/Cocoa/etc backends actually do what the core tests say they should. The contribution guide has a section on how the testbed tests work. In this case, we have a test of visibility; but (a) it's evidently it's either incorrect, or not validating a case that it needs to; and (b) there isn't an analogous test of display=NONE, or a test of the interaction of display and visibility controls.
(3) I've read the GTK documentation and I can't find another solution. If the widget is hidden, we need to make it visible in order to measure it. I changed the implementation to avoid toggling the visibility if the widget is visible to avoid extra calls. Visually, I don't see any flickering with current implementations. But I'm not fluent enough in GTK to know if another approach exists.
Empirically, the way to validate this will be to instrument Toga's own code to see how many resize/recompute passes are occurring. The GTK container implementation has a do_size_allocate() method; this method is invoked every time a GTK-level resize is requested. If the number of calls to this method doesn't change as a result of toggling visibility during a resize, we can probably be safe in assuming that the toggle is just setting the flag to perform a resize (which will be set anyway as a result of recomputing the size of the widget), rather than actually performing a repaint.
Closing due to lack of response. If you intend to continue this work, let us know an we can re-open the PR.