Skip to content
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
merged 28 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 23 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 Jun 14, 2023
5aac223
Correct docs typo.
freakboy3742 Jun 14, 2023
305a3f2
Update core tests for SplitContainer.
freakboy3742 Jun 14, 2023
89686bd
Cocoa SplitContainer to 100% coverage.
freakboy3742 Jun 16, 2023
d9e5737
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jun 18, 2023
134a3ff
Cleanups to Cocoa implementation.
freakboy3742 Jun 18, 2023
fd6d44b
GTK SplitContainer tests to 100%.
freakboy3742 Jun 18, 2023
8d64f1d
Fix markup issues with splitcontainer image.
freakboy3742 Jun 19, 2023
dbc6887
Update some test comments.
freakboy3742 Jun 19, 2023
8b83e64
Improve protection against double-setting the split.
freakboy3742 Jun 19, 2023
5c4cd00
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jun 20, 2023
a324c37
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jun 22, 2023
4195a8f
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jun 26, 2023
1b224c6
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jun 26, 2023
0414244
Merge branch 'audit-scrollcontainer' into audit-splitcontainer
freakboy3742 Jul 12, 2023
e7c945b
Add documentation for the constants module.
freakboy3742 Jul 12, 2023
b9cafba
Corrected inconsistent page titles.
freakboy3742 Jul 12, 2023
7374d4c
Apply suggestions from code review
freakboy3742 Jul 12, 2023
3399e45
Use direction constant consistently.
freakboy3742 Jul 12, 2023
87b8500
Allow empty content in split containers, and allow flex values <1
freakboy3742 Jul 12, 2023
88f1322
Update docs reference to <1 flex handling.
freakboy3742 Jul 12, 2023
489b0eb
Merge branch 'main' into audit-splitcontainer
mhsmith Jul 12, 2023
bd3f467
Documentation cleanups
mhsmith Jul 12, 2023
a9d06a4
Replace curved apostrophe which caused spell checking error
mhsmith Jul 12, 2023
dfb8361
Add splitcontainer example app
mhsmith Jul 12, 2023
b448213
Removed some redundant test cases.
freakboy3742 Jul 12, 2023
ae34946
Winforms to 100%
mhsmith Jul 15, 2023
a19aa46
Change mobile splitcontainer tests from `skip` back to `xfail`
mhsmith Jul 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/1984.feature.rst
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.
1 change: 1 addition & 0 deletions changes/1984.removal.1.rst
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.
1 change: 1 addition & 0 deletions changes/1984.removal.2.rst
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.
33 changes: 19 additions & 14 deletions cocoa/src/toga_cocoa/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ def isFlipped(self) -> bool:


class BaseContainer:
def __init__(self, content=None, on_refresh=None):
def __init__(self, on_refresh=None):
"""A base class for macOS containers.

:param content: The widget impl that is the container's initial content.
:param on_refresh: The callback to be notified when this container's layout is
refreshed.
"""
self._content = content
self._content = None
self.on_refresh = on_refresh
# macOS always renders at 96dpi. Scaling is handled
# transparently at the level of the screen compositor.
Expand Down Expand Up @@ -60,20 +59,16 @@ def refreshed(self):


class MinimumContainer(BaseContainer):
def __init__(self, content=None):
"""A container for evaluating the minumum possible size for a layout

:param content: The widget impl that is the container's initial content.
"""
super().__init__(content=content)
def __init__(self):
"""A container for evaluating the minumum possible size for a layout"""
super().__init__()
self.width = 0
self.height = 0


class Container(BaseContainer):
def __init__(
self,
content=None,
min_width=100,
min_height=100,
layout_native=None,
Expand All @@ -83,7 +78,6 @@ def __init__(

Creates and enforces minimum size constraints on the container widget.

:param content: The widget impl that is the container's initial content.
:param min_width: The minimum width to enforce on the container
:param min_height: The minimum height to enforce on the container
:param layout_native: The native widget that should be used to provide size
Expand All @@ -94,7 +88,8 @@ def __init__(
:param on_refresh: The callback to be notified when this container's layout is
refreshed.
"""
super().__init__(content=content, on_refresh=on_refresh)
super().__init__(on_refresh=on_refresh)

self.native = TogaView.alloc().init()
self.layout_native = self.native if layout_native is None else layout_native

Expand Down Expand Up @@ -133,8 +128,18 @@ def width(self):
def height(self):
return self.layout_native.frame.size.height

def set_min_width(self, width):
@property
def min_width(self):
return self._min_width_constraint.constant

@min_width.setter
def min_width(self, width):
self._min_width_constraint.constant = width

def set_min_height(self, height):
@property
def min_height(self):
return self._min_height_constraint.constant

@min_height.setter
def min_height(self, height):
self._min_height_constraint.constant = height
3 changes: 0 additions & 3 deletions cocoa/src/toga_cocoa/widgets/multilinetextinput.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ def create(self):
self.native.autohidesScrollers = False
self.native.borderType = NSBezelBorder

# Disable all autolayout functionality on the outer widget
self.native.translatesAutoresizingMaskIntoConstraints = False

# Create the actual text widget
self.native_text = TogaTextView.alloc().init()
self.native_text.interface = self.interface
Expand Down
164 changes: 106 additions & 58 deletions cocoa/src/toga_cocoa/widgets/splitcontainer.py
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
Comment on lines 101 to +104
Copy link
Member

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 update min_width and min_height, but I don't see any equivalent in Cocoa. Doesn't that mean that it'll always use the minimum size stored in set_content, which won't reflect any later internal changes to the content?

Copy link
Member Author

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:

  • explicitly check the case where the sub-container is > and < MIN in the given axis - the current test only validates "can't make the splitter smaller than the minimum"; and
  • check the case where the layout of the sub-container is altered without changing the root widget that is associated with the split container.

The second case can't really be checked until #2020; so I agree it makes sense to defer until then.

Copy link
Member

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.

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)
4 changes: 2 additions & 2 deletions cocoa/src/toga_cocoa/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ def show(self):
self.interface.content,
MinimumContainer(),
)
self.container.set_min_width(self.interface.content.layout.width)
self.container.set_min_height(self.interface.content.layout.height)
self.container.min_width = self.interface.content.layout.width
self.container.min_height = self.interface.content.layout.height

# Refresh with the actual viewport to do the proper rendering.
self.interface.content.refresh()
Expand Down
21 changes: 21 additions & 0 deletions cocoa/tests_backend/widgets/splitcontainer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import asyncio

from toga_cocoa.libs import NSSplitView

from .base import SimpleProbe


class SplitContainerProbe(SimpleProbe):
native_class = NSSplitView

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
8 changes: 8 additions & 0 deletions core/src/toga/constants/__init__.py
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
25 changes: 13 additions & 12 deletions core/src/toga/widgets/divider.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
from __future__ import annotations

from toga.constants import Direction

from .base import Widget


class Divider(Widget):
HORIZONTAL = 0
VERTICAL = 1
HORIZONTAL = Direction.HORIZONTAL
VERTICAL = Direction.VERTICAL

def __init__(
self,
id=None,
style=None,
direction=HORIZONTAL,
direction: Direction = HORIZONTAL,
):
"""Create a new divider line.

Inherits from :class:`toga.Widget`.

:param id: The ID for the widget.
:param style: A style object. If no style is provided, a default style
will be applied to the widget.
:param direction: The direction in which the divider will be drawn.
Defaults to ``Divider.HORIZONTAL``
:param style: A style object. If no style is provided, a default style will be
applied to the widget.
:param direction: The direction in which the divider will be drawn. Either
:attr:`~toga.constants.Direction.HORIZONTAL` or
:attr:`~toga.constants.Direction.VERTICAL`; defaults to
:attr:`~toga.constants.Direction.HORIZONTAL`
"""
super().__init__(id=id, style=style)

Expand All @@ -47,11 +51,8 @@ def focus(self):
pass

@property
def direction(self):
"""The direction in which the visual separator will be drawn.

:returns: ``Divider.HORIZONTAL`` or ``Divider.VERTICAL``
"""
def direction(self) -> Direction:
"""The direction in which the visual separator will be drawn."""
return self._impl.get_direction()

@direction.setter
Expand Down
Loading