Skip to content

Ensure Window always has a content widget #3478

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Ensure Window always has a content widget #3478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/2818.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A Window is now guaranteed to have a content widget. If no content is provided at time of construction, an empty Box widget will be added as the content.
4 changes: 2 additions & 2 deletions core/src/toga/window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ def __init__(
App.app.windows.add(self)

# If content has been provided, set it
if content:
self.content = content
# Otherwise, set content to an empty Box widget
self.content = content if content else toga.Box()
Comment on lines +248 to +249
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has resulted in a few branch coverage failures. They could be fixed by removing the if check at each branch and assuming that content is never None.

If we're committing to that invariant, then we should also update the content setter to interpret None in the same way as the constructor does. However, we're not sure whether this invariant is actually a good idea, as it prevents content=None from being round-tripped.

The presence of a default Box in the app's main window has obviously also complicated many of the tests, even if they didn't involve the main window directly.

Let's leave this PR open to discuss this question, and consider alternative approaches to the original issue in #2818.

Copy link
Contributor Author

@jgao8 jgao8 May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative approach in #3479, which is now merged.


self.on_close = on_close

Expand Down
79 changes: 45 additions & 34 deletions core/tests/widgets/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_add_child(app, widget):
assert_action_performed_with(window.content, "refresh")

# App's widget index has been updated
assert len(app.widgets) == 2
assert len(app.widgets) == 3
assert app.widgets["widget_id"] == widget
assert app.widgets["child_id"] == child

Expand Down Expand Up @@ -246,8 +246,8 @@ def test_add_multiple_children(app, widget):
child2 = ExampleLeafWidget(id="child2_id")
child3 = ExampleLeafWidget(id="child3_id")

# App's widget index only contains the parent
assert dict(app.widgets) == {"widget_id": widget}
# App's widget index contains the parent
assert app.widgets["widget_id"] == widget

# Add the children.
widget.add(child1, child2, child3)
Expand Down Expand Up @@ -286,6 +286,7 @@ def test_add_multiple_children(app, widget):
"child1_id": child1,
"child2_id": child2,
"child3_id": child3,
app.main_window.content.id: app.main_window.content,
}

# Window's widget index has been updated
Expand Down Expand Up @@ -449,6 +450,7 @@ def test_insert_child(app, widget):
assert dict(app.widgets) == {
"widget_id": widget,
"child_id": child,
app.main_window.content.id: app.main_window.content,
}

# Window's widget index has been updated
Expand Down Expand Up @@ -478,8 +480,8 @@ def test_insert_position(app, widget):
child2 = ExampleLeafWidget(id="child2_id")
child3 = ExampleLeafWidget(id="child3_id")

# App's widget index only contains the parent
assert dict(app.widgets) == {"widget_id": widget}
# App's widget index contains the parent
assert app.widgets["widget_id"] == widget

# Windows's widget index only contains the parent
assert dict(window.widgets) == {"widget_id": widget}
Expand Down Expand Up @@ -522,6 +524,7 @@ def test_insert_position(app, widget):
"child1_id": child1,
"child2_id": child2,
"child3_id": child3,
app.main_window.content.id: app.main_window.content,
}

# Window's widget index has been updated
Expand Down Expand Up @@ -551,8 +554,8 @@ def test_insert_bad_position(app, widget):
# Create a child widget
child = ExampleLeafWidget(id="child_id")

# App's widget index only contains the parent
assert dict(app.widgets) == {"widget_id": widget}
# App's widget index contains the parent
assert app.widgets["widget_id"] == widget

# Window's widget index only contains the parent
assert dict(window.widgets) == {"widget_id": widget}
Expand Down Expand Up @@ -582,6 +585,7 @@ def test_insert_bad_position(app, widget):
assert dict(app.widgets) == {
"widget_id": widget,
"child_id": child,
app.main_window.content.id: app.main_window.content,
}

# Window's widget index has been updated
Expand Down Expand Up @@ -711,7 +715,11 @@ def test_remove_child(app, widget):
assert child.parent == widget
assert child.app == app
assert child.window == window
assert dict(app.widgets) == {"widget_id": widget, "child_id": child}
assert dict(app.widgets) == {
"widget_id": widget,
"child_id": child,
app.main_window.content.id: app.main_window.content,
}
assert dict(window.widgets) == {"widget_id": widget, "child_id": child}

# Remove the child
Expand All @@ -726,7 +734,10 @@ def test_remove_child(app, widget):
assert child.window is None

# child widget no longer exists in the app or widgets registries.
assert dict(app.widgets) == {"widget_id": widget}
assert dict(app.widgets) == {
"widget_id": widget,
app.main_window.content.id: app.main_window.content,
}
assert dict(window.widgets) == {"widget_id": widget}

# The impl's remove_child has been invoked
Expand Down Expand Up @@ -938,7 +949,7 @@ def test_remove_from_non_parent(widget):

def test_set_app(app, widget):
"""A widget can be assigned to an app."""
assert len(app.widgets) == 0
assert len(app.widgets) == 1

# Assign the widget to an app
widget.app = app
Expand All @@ -947,7 +958,7 @@ def test_set_app(app, widget):
assert widget.app == app

# The widget doesn't appear in the index, as it's not on a window
assert len(app.widgets) == 0
assert len(app.widgets) == 1
assert "widget_id" not in app.widgets

# The impl has had its app property set.
Expand All @@ -958,7 +969,7 @@ def test_set_app(app, widget):
window.content = widget

# The widget is now in the app index.
assert len(app.widgets) == 1
assert len(app.widgets) == 2
assert app.widgets["widget_id"] == widget


Expand All @@ -970,7 +981,7 @@ def test_set_app_with_children(app, widget):
child3 = ExampleLeafWidget(id="child3_id")
widget.add(child1, child2, child3)

assert len(app.widgets) == 0
assert len(app.widgets) == 1

# Assign the widget as window content
window = toga.Window()
Expand All @@ -985,7 +996,7 @@ def test_set_app_with_children(app, widget):
assert child3.app == app

# The widget index has been updated
assert len(app.widgets) == 4
assert len(app.widgets) == 5
assert app.widgets["widget_id"] == widget
assert app.widgets["child1_id"] == child1
assert app.widgets["child2_id"] == child2
Expand All @@ -1000,7 +1011,7 @@ def test_set_app_with_children(app, widget):

def test_set_same_app(app, widget):
"""A widget can be re-assigned to the same app."""
assert len(app.widgets) == 0
assert len(app.widgets) == 1

# Assign the widget to an app
widget.app = app
Expand All @@ -1017,7 +1028,7 @@ def test_set_same_app(app, widget):

def test_reset_app(app, widget):
"""A widget can be re-assigned to no app."""
assert len(app.widgets) == 0
assert len(app.widgets) == 1

# Assign the widget to an app
widget.app = app
Expand All @@ -1032,7 +1043,7 @@ def test_reset_app(app, widget):
assert widget.app is None

# The widget index has been updated
assert len(app.widgets) == 0
assert len(app.widgets) == 1

# The impl has had its app property set.
assert attribute_value(widget, "app") is None
Expand All @@ -1043,14 +1054,14 @@ def test_set_new_app(app, widget):
# Assign the widget to an app. It won't appear in the registry, as
# it hasn't been assigned to a window
widget.app = app
assert len(app.widgets) == 0
assert len(app.widgets) == 1

# Reset the event log so we know the new events
EventLog.reset()

# Create a new app
new_app = toga.App("Test App", "org.beeware.toga.test-app")
assert len(new_app.widgets) == 0
assert len(new_app.widgets) == 1

# Assign the widget to the same app
widget.app = new_app
Expand All @@ -1059,9 +1070,9 @@ def test_set_new_app(app, widget):
assert widget.app == new_app

# The widget still doesn't appear in a widget index.
assert len(app.widgets) == 0
assert len(app.widgets) == 1
assert "widget_id" not in app.widgets
assert len(new_app.widgets) == 0
assert len(new_app.widgets) == 1
assert "widget_id" not in new_app.widgets

# The impl has had its app property set.
Expand All @@ -1071,7 +1082,7 @@ def test_set_new_app(app, widget):
def test_set_window(widget):
"""A widget can be assigned to a window."""
window = toga.Window()
assert len(window.widgets) == 0
assert len(window.widgets) == 1
assert widget.window is None

# Assign the widget to a window
Expand All @@ -1081,7 +1092,7 @@ def test_set_window(widget):
assert widget.window == window

# Window Widget registry has been updated
assert len(window.widgets) == 1
assert len(window.widgets) == 2
assert window.widgets["widget_id"] == widget


Expand All @@ -1094,8 +1105,8 @@ def test_set_window_with_children(app, widget):
widget.add(child1, child2, child3)

window = toga.Window()
assert len(app.widgets) == 0
assert len(window.widgets) == 0
assert len(app.widgets) == 2
assert len(window.widgets) == 1
assert widget.window is None
assert child1.window is None
assert child2.window is None
Expand All @@ -1111,8 +1122,8 @@ def test_set_window_with_children(app, widget):
assert child3.window == window

# Widget registry has been updated
assert len(app.widgets) == 4
assert len(window.widgets) == 4
assert len(app.widgets) == 6
assert len(window.widgets) == 5
assert window.widgets["widget_id"] == widget
assert window.widgets["child1_id"] == child1
assert window.widgets["child2_id"] == child2
Expand All @@ -1122,12 +1133,12 @@ def test_set_window_with_children(app, widget):
def test_reset_window(widget):
"""A widget can be assigned to a different window."""
window = toga.Window()
assert len(window.widgets) == 0
assert len(window.widgets) == 1
assert widget.window is None

# Assign the widget to a window
widget.window = window
assert len(window.widgets) == 1
assert len(window.widgets) == 2

# Create a new window
new_window = toga.Window()
Expand All @@ -1139,20 +1150,20 @@ def test_reset_window(widget):
assert widget.window == new_window

# Window Widget registry has been updated
assert len(window.widgets) == 0
assert len(new_window.widgets) == 1
assert len(window.widgets) == 1
assert len(new_window.widgets) == 2
assert new_window.widgets["widget_id"] == widget


def test_unset_window(widget):
"""A widget can be assigned to no window."""
window = toga.Window()
assert len(window.widgets) == 0
assert len(window.widgets) == 1
assert widget.window is None

# Assign the widget to a window
widget.window = window
assert len(window.widgets) == 1
assert len(window.widgets) == 2

# Assign the widget to no window
widget.window = None
Expand All @@ -1161,7 +1172,7 @@ def test_unset_window(widget):
assert widget.window is None

# Window Widget registry has been updated
assert len(window.widgets) == 0
assert len(window.widgets) == 1


@pytest.mark.parametrize(
Expand Down
2 changes: 1 addition & 1 deletion core/tests/window/test_document_window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def test_create(app):
window = doc.main_window

assert window.app == app
assert window.content is None
assert window.content is not None
# Document reference is preserved
assert window.doc == doc

Expand Down
Loading
Loading