-
-
Notifications
You must be signed in to change notification settings - Fork 685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[widget Audit] toga.SplitContainer #1984
Merged
freakboy3742
merged 28 commits into
beeware:main
from
freakboy3742:audit-splitcontainer
Jul 17, 2023
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
b9a1a38
Initial update to SplitContainer docs and API.
freakboy3742 5aac223
Correct docs typo.
freakboy3742 305a3f2
Update core tests for SplitContainer.
freakboy3742 89686bd
Cocoa SplitContainer to 100% coverage.
freakboy3742 d9e5737
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 134a3ff
Cleanups to Cocoa implementation.
freakboy3742 fd6d44b
GTK SplitContainer tests to 100%.
freakboy3742 8d64f1d
Fix markup issues with splitcontainer image.
freakboy3742 dbc6887
Update some test comments.
freakboy3742 8b83e64
Improve protection against double-setting the split.
freakboy3742 5c4cd00
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 a324c37
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 4195a8f
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 1b224c6
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 0414244
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 e7c945b
Add documentation for the constants module.
freakboy3742 b9cafba
Corrected inconsistent page titles.
freakboy3742 7374d4c
Apply suggestions from code review
freakboy3742 3399e45
Use direction constant consistently.
freakboy3742 87b8500
Allow empty content in split containers, and allow flex values <1
freakboy3742 88f1322
Update docs reference to <1 flex handling.
freakboy3742 489b0eb
Merge branch 'main' into audit-splitcontainer
mhsmith bd3f467
Documentation cleanups
mhsmith a9d06a4
Replace curved apostrophe which caused spell checking error
mhsmith dfb8361
Add splitcontainer example app
mhsmith b448213
Removed some redundant test cases.
freakboy3742 ae34946
Winforms to 100%
mhsmith a19aa46
Change mobile splitcontainer tests from `skip` back to `xfail`
mhsmith File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
The SplitContainer widget now has 100% test coverage, and complete API documentation. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Support for SplitContainers with more than 2 panels of content has been removed. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Support for 3-tuple form of specifying SplitContainer items, used to prevent panels from resizing, has been removed. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,84 +1,132 @@ | ||
from rubicon.objc import objc_method, objc_property | ||
from rubicon.objc import SEL, objc_method, objc_property | ||
from travertino.size import at_least | ||
|
||
from toga_cocoa.container import Container | ||
from toga_cocoa.libs import NSObject, NSSize, NSSplitView | ||
from toga.constants import Direction | ||
from toga_cocoa.container import Container, MinimumContainer | ||
from toga_cocoa.libs import NSSplitView | ||
|
||
from .base import Widget | ||
|
||
|
||
class TogaSplitViewDelegate(NSObject): | ||
class TogaSplitView(NSSplitView): | ||
interface = objc_property(object, weak=True) | ||
impl = objc_property(object, weak=True) | ||
|
||
@objc_method | ||
def splitView_resizeSubviewsWithOldSize_(self, view, size: NSSize) -> None: | ||
# Turn all the weights into a fraction of 1.0 | ||
total = sum(self.interface._weight) | ||
self.interface._weight = [weight / total for weight in self.interface._weight] | ||
|
||
# Mark the subviews as needing adjustment | ||
view.adjustSubviews() | ||
|
||
# Set the splitter positions based on the new weight fractions. | ||
cumulative = 0.0 | ||
if self.interface.direction == self.interface.VERTICAL: | ||
for i, weight in enumerate(self.interface._weight[:-1]): | ||
cumulative += weight | ||
view.setPosition(view.frame.size.width * cumulative, ofDividerAtIndex=i) | ||
else: | ||
for i, weight in enumerate(self.interface._weight[:-1]): | ||
cumulative += weight | ||
view.setPosition( | ||
view.frame.size.height * cumulative, ofDividerAtIndex=i | ||
) | ||
|
||
@objc_method | ||
def splitViewDidResizeSubviews_(self, notification) -> None: | ||
# If the window is actually visible, and the split has moved, | ||
# a resize of all the content panels is required. The refresh | ||
# needs to be directed at the root container holding the widget, | ||
# as the splitview may not be the root container. | ||
if self.interface.window and self.interface.window._impl.native.isVisible: | ||
self.interface.refresh() | ||
self.impl.on_resize() | ||
# If the split has moved, a resize of all the content panels is required. | ||
for container in self.impl.sub_containers: | ||
container.content.interface.refresh() | ||
|
||
# Apply any pending split | ||
self.performSelector( | ||
SEL("applySplit"), | ||
withObject=None, | ||
afterDelay=0, | ||
) | ||
|
||
class SplitContainer(Widget): | ||
"""Cocoa SplitContainer implementation. | ||
@objc_method | ||
def applySplit(self) -> None: | ||
if self.impl._split_proportion: | ||
if self.interface.direction == self.interface.VERTICAL: | ||
position = self.impl._split_proportion * self.frame.size.width | ||
else: | ||
position = self.impl._split_proportion * self.frame.size.height | ||
|
||
self.setPosition(position, ofDividerAtIndex=0) | ||
self.impl._split_proportion = None | ||
|
||
Todo: | ||
* update the minimum width of the whole SplitContainer based on the content of its sub layouts. | ||
""" | ||
|
||
class SplitContainer(Widget): | ||
def create(self): | ||
self.native = NSSplitView.alloc().init() | ||
self.native = TogaSplitView.alloc().init() | ||
self.native.interface = self.interface | ||
self.native.impl = self | ||
self.native.delegate = self.native | ||
|
||
self.sub_containers = [Container(), Container()] | ||
self.native.addSubview(self.sub_containers[0].native) | ||
self.native.addSubview(self.sub_containers[1].native) | ||
|
||
self.delegate = TogaSplitViewDelegate.alloc().init() | ||
self.delegate.interface = self.interface | ||
self.delegate.impl = self | ||
self.native.delegate = self.delegate | ||
self._split_proportion = None | ||
|
||
# Add the layout constraints | ||
self.add_constraints() | ||
|
||
def add_content(self, position, widget, flex): | ||
# TODO: add flex option to the implementation | ||
container = Container(content=widget) | ||
|
||
# Turn the autoresizing mask on the widget into constraints. | ||
# This makes the widget fill the available space inside the | ||
# SplitContainer. | ||
# FIXME Use Constraints to enforce min width and height of the widgets otherwise width of 0 is possible. | ||
container.native.translatesAutoresizingMaskIntoConstraints = True | ||
self.native.addSubview(container.native) | ||
def set_bounds(self, x, y, width, height): | ||
super().set_bounds(x, y, width, height) | ||
for container in self.sub_containers: | ||
if container.content: | ||
container.content.interface.refresh() | ||
|
||
# Apply any pending split | ||
self.native.performSelector( | ||
SEL("applySplit"), | ||
withObject=None, | ||
afterDelay=0, | ||
) | ||
|
||
def set_content(self, content, flex): | ||
# Clear any existing content | ||
for container in self.sub_containers: | ||
container.content = None | ||
|
||
for index, widget in enumerate(content): | ||
# Compute the minimum layout for the content | ||
if widget: | ||
widget.interface.style.layout(widget.interface, MinimumContainer()) | ||
min_width = widget.interface.layout.width | ||
min_height = widget.interface.layout.height | ||
|
||
# Create a container with that minimum size, and assign the widget as content | ||
self.sub_containers[index].min_width = min_width | ||
self.sub_containers[index].min_height = min_height | ||
|
||
self.sub_containers[index].content = widget | ||
else: | ||
self.sub_containers[index].min_width = 0 | ||
self.sub_containers[index].min_height = 0 | ||
|
||
# We now know the initial positions of the split. However, we can't *set* the | ||
# because Cocoa requires a pixel position, and the widget isn't visible yet. | ||
# So - store the split; and when have a displayed widget, apply that proportion. | ||
self._split_proportion = flex[0] / sum(flex) | ||
|
||
def get_direction(self): | ||
return Direction.VERTICAL if self.native.isVertical() else Direction.HORIZONTAL | ||
|
||
def set_direction(self, value): | ||
self.native.vertical = value | ||
|
||
def on_resize(self): | ||
pass | ||
|
||
def rehint(self): | ||
self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH) | ||
self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT) | ||
# This is a SWAG (scientific wild-ass guess). There doesn't appear to be | ||
# an actual API to get the true size of the splitter. 10px seems enough. | ||
SPLITTER_WIDTH = 10 | ||
if self.interface.direction == self.interface.HORIZONTAL: | ||
# When the splitter is horizontal, the splitcontainer must be | ||
# at least as wide as it's widest sub-container, and at least | ||
# as tall as the minimum height of all subcontainers, plus the | ||
# height of the splitter itself. Enforce a minimum size in both | ||
# axies | ||
min_width = self.interface._MIN_WIDTH | ||
min_height = 0 | ||
for sub_container in self.sub_containers: | ||
min_width = max(min_width, sub_container.min_width) | ||
min_height += sub_container.min_height | ||
|
||
min_height = max(min_height, self.interface._MIN_HEIGHT) + SPLITTER_WIDTH | ||
else: | ||
# When the splitter is vertical, the splitcontainer must be | ||
# at least as tall as it's tallest sub-container, and at least | ||
# as wide as the minimum width of all subcontainers, plus the | ||
# width of the splitter itself. | ||
min_width = 0 | ||
min_height = self.interface._MIN_HEIGHT | ||
for sub_container in self.sub_containers: | ||
min_width += sub_container.min_width | ||
min_height = max(min_height, sub_container.min_height) | ||
|
||
min_width = max(min_width, self.interface._MIN_WIDTH) + SPLITTER_WIDTH | ||
|
||
self.interface.intrinsic.width = at_least(min_width) | ||
self.interface.intrinsic.height = at_least(min_height) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import asyncio | ||
|
||
from toga_cocoa.libs import NSSplitView | ||
|
||
from .base import SimpleProbe | ||
|
||
|
||
class SplitContainerProbe(SimpleProbe): | ||
native_class = NSSplitView | ||
border_size = 0 | ||
direction_change_preserves_position = False | ||
|
||
def move_split(self, position): | ||
self.native.setPosition(position, ofDividerAtIndex=0) | ||
|
||
async def wait_for_split(self): | ||
sub1 = self.impl.sub_containers[0].native.frame.size | ||
position = sub1.height, sub1.width | ||
current = None | ||
while position != current: | ||
position = current | ||
await asyncio.sleep(0.05) | ||
current = sub1.height, sub1.width |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,9 @@ | ||
from enum import Enum | ||
|
||
from travertino.constants import * # noqa: F401, F403 | ||
|
||
|
||
class Direction(Enum): | ||
"The direction a given property should act" | ||
HORIZONTAL = 0 | ||
VERTICAL = 1 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether this code is properly verified by the testbed, and it isn't easy to test manually until minimum window size is working. So maybe any further work in this area should be deferred until #2020. However, I have a couple of comments:
This code is very similar between Cocoa and GTK, so it should be factored out into a function in the interface layer.
Also, I see that the GTK version calls
recompute
to updatemin_width
andmin_height
, but I don't see any equivalent in Cocoa. Doesn't that mean that it'll always use the minimum size stored inset_content
, which won't reflect any later internal changes to the content?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed there's a lot of similarity here; AFAIR, the discrepancy is because of GTK's "deferred" layout scheme. In Cocoa, by the time rehint() has been called, we know content has been added and Pack has been called to update minimum sizes; on GTK, Pack isn't invoked until the layout is actually underway, and it's done "top down" in the layout tree, so the layout of the children of the splitter haven't necessarily been computed yet.
I'm fairly sure you're also right that this will fall apart in the cases that #2020 fixes - i.e., the case where the root content of the split containers doesn't change, but there is a layout altering change in that content that changes the minimum.
There is a test for enforcing minimum sizes on containers, but it's probably not as thorough as it could be - it needs to:
The second case can't really be checked until #2020; so I agree it makes sense to defer until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've added a link to this discussion to #2020.