Cocoa Button Cramped with Parent Not In Layout

[johnzhou721]

Describe the bug

import os


import toga
from toga.style import Pack
from toga.style.pack import CENTER

def button_handler(widget):
    print("Button clicked!")

def button_handler2(widget):
    print("Button2 clicked!")

def build(app):
    box = toga.Box()
    
    button = toga.Button('Click Me', on_press=button_handler, style=Pack(margin=5))
    
    box.add(button)
    
    return button

def main():
    return toga.App('Simple Button App', 'org.example.simplebutton', startup=build)

if __name__ == '__main__':
    app = main()
    app.main_loop()

So over here the content should be a button that fills the window but we instead get this cramped:

Steps to reproduce

• Install Toga 0.5.2
• Run the above repro
• See cramped button

Expected behavior

Something like this, i.e. box.add(button) does not impact layout at all.

Screenshots

No response

Environment

• macOS Sonoma
• Toga 0.5.2
• Python 3.10.14
• Rubicon-objc 0.5.1

Logs

No response

Additional context

No response

[freakboy3742]

This isn't an error with layout math - it's a bug in error handling.

Your example adds the button to a box, then returns the button as content. That means the button has a parent... but the parent isn't involved in the layout. I'm guessing that then manifests as the layout algorithm determining that the parent has zero size, and the button falls back to bare minimum size.

However, that's just a symptom of the real issue. We have error handling to check ensure that if a child is added to a widget, parents are re-assigned; but if you add a child as the content of a window, there's no check that the widget has no parent, and no attempt made to remove the child from it's parent.

There are two possible fixes:

1. We raise an error in Window.content if the widget being used as content has a parent.
2. We orphan the widget (i.e., remove it from the parent) if it is added as content of a window.

I'm not sure which of these would be better. (2) provides the path that is most like to run without error - but this sort of thing is more likely to be done in error than by intention. If you're making this kind of mistake, maybe an explicit error from (1) is the better outcome.

[johnzhou721]

FWIW -- I will open a PR to go with 1) but instead of raising, use an assert since it's consistent with how widget parents are handled; this sort of stuff is likely a user's error, so no explicit test will get added and the changenot will be misc.

[johnzhou721]

Hold on... nevermind, that's just an extra assert in the impls, the parent's actually reassigned

I'll go with 2) and add a test

[edit] for clarity, the rationale is to stay consistent with widget parenting.