toga icon indicating copy to clipboard operation
toga copied to clipboard

Ensure that windows always have content

Open freakboy3742 opened this issue 7 months ago • 1 comments

What is the problem or limitation you are having?

At present, window.content can be either a widget, or None. It would simplify some code paths (and avoid some bugs) if we could always guarantee that a window had some content, even if it is an empty box.

Describe the solution you'd like

If a window is constructed without an explicit content object, a toga.Box should be created and installed as content.

Describe alternatives you've considered

Status quo - make a policy decision that we're going to allow truly empty windows, and stick with that.

Additional context

This originally came up in the context of #2818, which was an actual bug caused by a window not having content. #2473 also identified some edge cases that would be made more simple if windows always had content.

However, this is a case where the fix is trivial, but the impact on testing is huge. #3478 was an attempt to fix this problem - and although the "raw fix" is only 2 lines, it introduced dozens of cascading changes into the test suite, because any test case that counts widgets in the widget registry now has an additional widget. This is especially problematic when the test case has an additional window, and doesn't actually care about the content in the main window.

So - we have three options:

  1. Keep the status quo, and accept that windows might not have content
  2. Modify the test suite to accomodate the extra widget - essentially bumping every widget count by 1, possibly documenting where the extra widget comes from in situations where it may not be obvious
  3. Modify the implementation slightly so it is possible (maybe even undocumented) that None is a valid value if it is explicit.

(3) had the least impact on the test suite, but doesn't really solve the problem because we'd still need to accomodate the is None case everywhere (or, at least, a lot of places).

It might be possible minimise the impact of (2) by changing some tests to actually use the main window, rather than a secondary window.

freakboy3742 avatar May 21 '25 12:05 freakboy3742

If we did this, then for consistency:

  • It should also apply to the content setter.
  • It should also apply to all other content attributes, such as in ScrollContainer.

This would make the content attribute a little more confusing, because None wouldn't be round trippable anymore. It's also a backward-incompatible change, because it would break any app code which is comparing content to None, as the current contract gives them a perfect right to do. So I'm leaning towards keeping the status quo.

mhsmith avatar May 21 '25 14:05 mhsmith