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.

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

All apps get a default content when no content is provided. #2844

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

All apps get a default content when no content is provided. #2844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8732539
All apps get a default content when no content is provided.
cgkoutzigiannis Sep 14, 2024
219c9d0
Add a new change file.
cgkoutzigiannis Sep 14, 2024
6e6915d
Fixed first few failing tests.
cgkoutzigiannis Sep 16, 2024
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 @@
When no content is provided in the Window object, default to empty Box.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When no content is provided in the Window object, default to empty Box.
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.

3 changes: 1 addition & 2 deletions core/src/toga/window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,7 @@ def __init__(
App.app.windows.add(self)

# If content has been provided, set it
if content:
self.content = content
self.content = content if content else toga.Box()

self.on_close = on_close

Expand Down
46 changes: 26 additions & 20 deletions core/tests/widgets/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,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 @@ -242,8 +242,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 widget == dict(app.widgets)["widget_id"]

# Add the children.
widget.add(child1, child2, child3)
Expand Down Expand Up @@ -277,12 +277,15 @@ def test_add_multiple_children(app, widget):
assert_action_performed_with(window.content, "refresh")

# App's widget index has been updated
assert dict(app.widgets) == {
"widget_id": widget,
"child1_id": child1,
"child2_id": child2,
"child3_id": child3,
}
assert (
dict(app.widgets).items()
>= {
"widget_id": widget,
"child1_id": child1,
"child2_id": child2,
"child3_id": child3,
}.items()
Copy link
Member

Choose a reason for hiding this comment

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

While this does pass, It's not a very robust test.

dict.items() will be converting the dictionary into a list of tuples; the list comparison will be trying to evaluate that the two lists sort in lexical order. In effect, this test asserts that the app.widgets list has, at the bare minimum, a single widget that is alphabetically sorted before "widget_id". This will evaluate as true in this case... but so would any app widget list that contains at least one item ({'foo': widget} would pass this test, for example, even though it doesn't contain any of the child widgets).

If the intention is to validate that "all of these objects in app.widgets", it would be better to evaluate each membership specifically; or, alternatively, use a subset-based check (possibly factored out as a standalone assertion).

Another approach in this case would be even simpler - use a reference to the literal "extra" widget that is causing an issue for the test. The extra widget that prevents the use of a literal == here is the content widget of the test app's main window - something that isn't involved in this test, but is in the app registry. Adding app.main_window.content.id: app.main_window.content, as an additional pair in the expected output is a reasonable approach in this case - we don't care about the specific ID or widget; we just care that the app's widget registry contains a reference to all known widgets - and the app's (default) content is one of those widgets.

)

# Window's widget index has been updated
assert dict(window.widgets) == {
Expand Down Expand Up @@ -417,8 +420,8 @@ def test_insert_child(app, widget):
# Create a child widget
child = ExampleLeafWidget(id="child_id")

# App's widget index only contains the parent
assert app.widgets["widget_id"] == widget
# App's widget index contains the parent
assert widget == dict(app.widgets)["widget_id"]
assert "child_id" not in app.widgets

# insert the child.
Expand All @@ -442,10 +445,13 @@ def test_insert_child(app, widget):
assert_action_performed_with(window.content, "refresh")

# App's widget index has been updated
assert dict(app.widgets) == {
"widget_id": widget,
"child_id": child,
}
assert (
dict(app.widgets).items()
>= {
"widget_id": widget,
"child_id": child,
}.items()
)

# Window's widget index has been updated
assert dict(window.widgets) == {
Expand Down Expand Up @@ -933,7 +939,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 @@ -942,7 +948,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 @@ -953,7 +959,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 @@ -965,7 +971,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 @@ -980,7 +986,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 Down
Loading