Set Content When Initializing Window [Cont.] #2586
Conversation
|
Will the doc string context changes update the ReadTheDocs? I haven't checked that on the list juuust in case. |
Cameronsplaze
changed the title
Yes: there's a readthedocs preview link at the bottom of the "checks" section below. And after the PR is merged, the changes will be made public under https://toga.readthedocs.io/en/latest/ . Note the URL says "latest" as opposed to "stable" – you can switch between them using the link at the bottom left on RTD. |
|
Cool, thanks! I didn't realize docs can be created on PR's, I really like that feature! Is there anything else you'd like changed with this PR while I'm here? |
core/src/toga/window.py
Outdated
| @@ -198,6 +199,8 @@ def __init__( | |||
|
|
|||
| self.on_close = on_close | |||
|
|
|||
| self._content = content | |||
There was a problem hiding this comment.
This needs to use the setter, not just set the internal property, as there is other housekeeping that needs to be performed.
core/tests/window/test_window.py
Outdated
| assert window_with_content.content.children[0] == label1 | ||
| finally: | ||
| window_with_content.close() | ||
| window_with_content = None |
There was a problem hiding this comment.
There's no need to do cleanup like this in the dummy backend - it doesn't leave any artefacts that could be a problem.
core/tests/window/test_window.py
Outdated
| assert window_no_content.content is None | ||
| finally: | ||
| window_no_content.close() | ||
| window_no_content = None |
There was a problem hiding this comment.
There's no need to test the "no content" option here - this is tested extensively by existing tests.
core/tests/window/test_window.py
Outdated
| assert window_with_content.content == content | ||
| # Make sure it still has that internal content: | ||
| assert len(window_with_content.content.children) == 1 | ||
| assert window_with_content.content.children[0] == label1 |
There was a problem hiding this comment.
The important thing to test here isn't the parenting of content - it's the app and window assignments on the content.
testbed/tests/test_window.py
Outdated
| assert window_no_content.content is None | ||
| finally: | ||
| window_no_content.close() | ||
| window_no_content = None |
There was a problem hiding this comment.
As with the core APIs test - there's no need to test the "no content" case; that's tested elsewhere.
testbed/tests/test_window.py
Outdated
| assert window_with_content.content == content | ||
| # Make sure it still has that internal content: | ||
| assert len(window_with_content.content.children) == 1 | ||
| assert window_with_content.content.children[0] == label1 |
There was a problem hiding this comment.
To verify this is working, the window needs to be shown, and a probe needs to wait for the window to be displayed so that a --slow invocation can visually verify the result.
changes/5753.feature.rst
Outdated
There was a problem hiding this comment.
Not sure where this changenote came from (or where 5753 comes from); this can be deleted.
(A continuation from #2314)
PR Checklist: