Implement the Scaffold layer for core and dummy

[johnzhou721]

Refs #4298.

This is a work-in-progress implementation. A broad checklist that I try to go off of:

• Note: Toolbars are going to be handled by scaffold logic on all platforms, even though for users it'll be just window toolbar, because when Sidebar and Option scaffolds are added, toolbars need to be managed by the scaffold implementation, so ti's better to be symmetrical.
• Main menus can be handled in scaffolds if appropriate; have a registry of scaffolds, and pass down changes in create_menu onto all scaffolds. But this will be handled later.
• Clearly separate container and window logic on platforms where this isn't done yet
  • Android
  • WinForms
• Track scaffold of each widget
• Add scaffold object in core, change backend API contract to set_scaffold
• Have scaffolds manage content setting
• Have scaffolds manage toolbar creation storage, but base scaffolds do not expose toolbar attribute
• Implement Dummy
  • Implement dummy by mocking toolbar creation and content setting; migrate content from window.py
  • Interact with scaffold on frame layout code in dummy
• Tests for core:
  • Verify setting scaffold works
  • Verify correct semantics for content and _scaffold
  • Verify correct content being hooked onto window
• Code review on Jun 1st
• Adapt each backend:
  • macOS
  • iOS
    • Move UINavigationController to scaffold
  • GTK
  • Qt
  • Android
    • Potential TODOs in restructuring for Material 3?
  • Web
  • Textual
• Change frame layout code to interact with scaffold.
• Testbed tests:
  • Verify directly setting scaffold works.
  • Update probes
  • Make sure toolbar creation is correctly coordinated:
    • Assignment of new scaffold
  • Nothing more; only existing tests need to be updated as they test toolbar behaviors

PR Checklist:

• I will abide by the BeeWare Code of Conduct
• I have read and have followed the CONTRIBUTING.md file
• This PR was generated or assisted using an AI tool

Assisted-by: GItHub Copilot; Google Gemini, ChatGPT (latest web)

[johnzhou721]

@freakboy3742 As I've aced my last final exam in school, I've began to work on this more. I'm mostly done with core now but have a question about tracking app and window on Widget.

app and window are directly assignable by the user on Widget... so it's really awkward, and there's a lot of duplciation of keeping track of both when app can just be derived from the window, esp. since set_app on widget is empty on all backends anyways. Is there any particular reason why we're not simplifying app to just be a getter-only for window.app?

The reason I'm talking about this is that I'm thinking of taking a more simplified approach to getting the scaffold — while it is possible to handle it in a similar manner as window and app tracking, i.e. assign scaffold to widget, I think just having a scaffold getter that is self.window.scaffold may be worthwhile. But that does introduce some assymmetry. as app and window are settable and documentd in the docstrings as such.

[freakboy3742]

@freakboy3742 As I've aced my last final exam in school, I've began to work on this more. I'm mostly done with core now but have a question about tracking app and window on Widget.

app and window are directly assignable by the user on Widget... so it's really awkward, and there's a lot of duplciation of keeping track of both when app can just be derived from the window, esp. since set_app on widget is empty on all backends anyways. Is there any particular reason why we're not simplifying app to just be a getter-only for window.app?

The main reason I can think of is that a widget can exist without necessarily being part of a window layout. I don't recall if there was a historical reason why a widget needed to be assigned to an app explicitly so that some "before the widget gets assigned to the window" handling could be done - but it's entirely possible that reason no longer exists (or never existed and we were over-complicating things).

The reason I'm talking about this is that I'm thinking of taking a more simplified approach to getting the scaffold — while it is possible to handle it in a similar manner as window and app tracking, i.e. assign scaffold to widget, I think just having a scaffold getter that is self.window.scaffold may be worthwhile. But that does introduce some assymmetry. as app and window are settable and documentd in the docstrings as such.

app and window need to be settable on widgets in the sense that there are parts of Toga that need to set them - but there's no expectation that a user would ever set them in practice. The setters for those are effectively "private", but there's no real way to declare a public getter and a private setter (other than not making the setter an actual setter).

Proxying scaffold retrieval through the window makes some sense - my only concern is whether that will remain accurate in future in more complicated situations where a second-level scaffold is allowed (e.g., the situations where tabs in an OptionScaffold themselves have scaffolds). However, that might be worth deferring to the future when we actually have that problem - as long as a widget can get it's scaffold, we're not saying anything about how it determines that value, so if it needs to change in future, it can.

[johnzhou721]

Proxying scaffold retrieval through the window makes some sense - my only concern is whether that will remain accurate in future in more complicated situations where a second-level scaffold is allowed (e.g., the situations where tabs in an OptionScaffold themselves have scaffolds). However, that might be worth deferring to the future when we actually have that problem - as long as a widget can get it's scaffold, we're not saying anything about how it determines that value, so if it needs to change in future, it can.

Well... SidebarScaffold in OptionScaffold having a side pane is effectively broken in older iOS version we need to support—as discussed in #4299... But that may as well be a separate issue.

But since I already suggested getting App from Window, we can take this a step further: track scaffold, get window from scaffold, and get app from window... this is going to make the codebase a lot easier to understand and will alleviate any nesting concerns. I'll try to implement that as part of this PR on my next cycle.

app and window need to be settable on widgets in the sense that there are parts of Toga that need to set them - but there's no expectation that a user would ever set them in practice. The setters for those are effectively "private", but there's no real way to declare a public getter and a private setter (other than not making the setter an actual setter).

The set behavior seems publically documented -- bug in docs?

[freakboy3742]

app and window need to be settable on widgets in the sense that there are parts of Toga that need to set them - but there's no expectation that a user would ever set them in practice. The setters for those are effectively "private", but there's no real way to declare a public getter and a private setter (other than not making the setter an actual setter).

The set behavior seems publically documented -- bug in docs?

Not so much a bug as something that is difficult to document. It's traversing the gap between "things that need to exist and be documented for internal use", and "things that the end user actually needs to care about". It's not wrong - that is how the setter works. But as an end user, you shouldn't ever need to care. Can we clean this up? Almost certainly.

[johnzhou721]

After thinking about this more: I don't think I'm going to try to refactor window and app in this PR, and I'm just going to track scaffold like we track Window and App previously. I'm making this decision because there's a widget registry for each window... so it's going to be hard to move things around (one can of course make a registry for each scaffold but doing all the bookkeeping and restructuring of getting widgets in each object is going to scope creep.)

TL;DR I'm going to track scaffold for each widget the same way app and window are tracked, to leave the door open for future nesting if needed and avoid scope creep.

[johnzhou721]

@freakboy3742 I'm done with core, dummy, and their tests; dependabot PRs are blocking CI, but I've manually tested with macOS 3.12 and 3.13 on my mac and they all work well.

I would appreciate a partial review of those parts.

[johnzhou721]

Note: After the latest commit this is still [1] ready for a partial review of core, dummy and their tests; I merely used scaffold to consistently interact with the layout code.

[1] — my last message sent prematurely; I've edited it to indicate this was ready for a partial review, but turns out I missed a change.
[2] EDIT: Android and Winforms code are just refactoring container out of the window logic

[johnzhou721]

Feel free to give the partial review suggested above, but note that I will not start backend work until #4275 is finished, because both this and #4275 touches iOS window and container logic.

[johnzhou721]

@freakboy3742 Touching base on this one---to be explicit, I need a partial review of core/ before continuing; sorry for my vague wording.

[freakboy3742]

From an initial light review - this looks like it's on the right track. The general shape of the Scaffold core API looks right; a few comments inline, but nothing too significant at this point. I'm sure more details will fall out of the iOS, and macOS backends.

The only detail of note at this point is that if preparatory work is needed (like the "has-a" vs "is-a" relationship for Android and Windows containers, that should be pulled out as a precursor PR. We can land that work indpendently, which makes it a lot easier to evaluate the actual scaffold changes from other housekeeping.

[johnzhou721]

@freakboy3742 This is ready for review now to merge into scaffolds branch. I've fixed up all the issues you mentioned and moved Android and WinForms priming to #4484.

FTR: As discussed over discord and elsewhere:

• A new branch has been established for Scaffolds since this is a major overhaul.
• Each platform and core will be merged separately into that branch before integration into main; CI failure is acceptable for merge, which is the point of a branch to allow for partial implementations.
• I will not be starting any other container or toolbar fixes for any platform before Scaffold finishes.

[johnzhou721]

The real place this triggers for me is the repeated widget.app = app; widget.window = window; widget.scaffold = scaffold pattern of logic. When a widget is assigned to a scaffold, there's only one possible window and app, so 3 assignments shouldn't be necessary.

One thing I've thought about is making app a dervied property from window and window a derived property from scaffold.

The issue preventing this was that the window kept a widget registry... so that needed to be handled separately, and we'd end up in a situation where we technically can make app derived... but is app derived from window or scaffold, if the scaffold is not attached to a window? I agree this is a relatively contrived case and that there's probably some opportunity for cleanup that can make this work easily, but I didn't want to think about the moving parts (pun intended) involved here for the sake of pragmatism.

There's also the need for test updates to explicitly associate stuff with a window instead of setting the window property, which is a lot of work. So I left it out.

I've made the requested changes that I agreed with, and am now waiting on some clarifications and discussions I've posed inline.

(I also apologize for replying to comments individually inline, as bunching them requires the Files Changed page which was quite difficult to go through. I replied "Done" to some things as I've just read that it was not permissible to click Resolve Conversation myself, so that it's clear what is still ongoing to be discussed here; let me know if this is unnecessary.)

---

EDIT: Also CI seems to be broken. I pushed very often to check if it was intermittent, but seems like it's a permanent issue now. I suspect it's the vcs_versioning fiesta -- can you merge main into scaffolds? After that, let me know and I'll merge scaffolds into this PR to see if things pass.

And of course, thanks kattni for handling the PR routing.

[johnzhou721]

Since we're going to track app and window and those are tracked separately with scaffolds, I'm going to be refactoring this into scaffold.content (and similar places).

[freakboy3742]

I agree this is a relatively contrived case and that there's probably some opportunity for cleanup that can make this work easily, but I didn't want to think about the moving parts (pun intended) involved here for the sake of pragmatism.

I'm ok to leave this as a pragmatic commission for now, and maybe address this in future as an independent cleanup pass that clarifies the API surface of widget/window to remove setters of things that users shouldn't be setting.

EDIT: Also CI seems to be broken. I pushed very often to check if it was intermittent, but seems like it's a permanent issue now. I suspect it's the vcs_versioning fiesta -- can you merge main into scaffolds? After that, let me know and I'll merge scaffolds into this PR to see if things pass.

Done - and I've merged this PR as well.

[freakboy3742]

(I also apologize for replying to comments individually inline, as bunching them requires the Files Changed page which was quite difficult to go through. I replied "Done" to some things as I've just read that it was not permissible to click Resolve Conversation myself, so that it's clear what is still ongoing to be discussed here; let me know if this is unnecessary.)

Addressing this one specifically: Replying to comments individually inline is fine - if you're responding to specific feedback about a specific line/section of code, the thread coming off that feedback is the right place to be posting. The fact that each "comment" turns into a separate email isn't a concern - yes, it's more emails, but that's not the thing that is a problem.

Broadly speaking, a thread on a single line/section should be a comment from A, then a reply from B, then a response to the reply from A, and so on. If ABABAB happens on multiple different lines of code, that's fine (and expected). An occasional ABBA is occasionally going to happen; that's OK as well. The problem is when the thread is ABBBBBBBBB - that's an indication that B hasn't thought through what they want to say before responding.

As for posting "Done" - that's fine (and can even be helpful if it's not obvious that whatever has been requested has actually been done. I will sometimes use a 👍 on a review comment instead of an explicit "done", unless there's some specific interesting detail about what "done" means (e.g., "I've done what you've asked, but it had this other consequence that might not be obvious")

[johnzhou721]

(I also apologize for replying to comments individually inline, as bunching them requires the Files Changed page which was quite difficult to go through. I replied "Done" to some things as I've just read that it was not permissible to click Resolve Conversation myself, so that it's clear what is still ongoing to be discussed here; let me know if this is unnecessary.)

Addressing this one specifically: Replying to comments individually inline is fine - if you're responding to specific feedback about a specific line/section of code, the thread coming off that feedback is the right place to be posting. The fact that each "comment" turns into a separate email isn't a concern - yes, it's more emails, but that's not the thing that is a problem.

Broadly speaking, a thread on a single line/section should be a comment from A, then a reply from B, then a response to the reply from A, and so on. If ABABAB happens on multiple different lines of code, that's fine (and expected). An occasional ABBA is occasionally going to happen; that's OK as well. The problem is when the thread is ABBBBBBBBB - that's an indication that B hasn't thought through what they want to say before responding.

As for posting "Done" - that's fine (and can even be helpful if it's not obvious that whatever has been requested has actually been done. I will sometimes use a 👍 on a review comment instead of an explicit "done", unless there's some specific interesting detail about what "done" means (e.g., "I've done what you've asked, but it had this other consequence that might not be obvious")

Thanks for being explicit with this concern; I understand now that the issue is avoidng unneccessary noise in terms of monologue, and appreciate the time you have taken to make this clear.

[johnzhou721]

I'm pretty sure I addressed most things here except for the double-content-setting issue, which is not a blocker; so I'm asking for another review now.

[freakboy3742]

Getting close to mergable; a couple more cleanups flagged inline.

[johnzhou721]

Thanks for flagging the issues, and for your patience on working this another round. I've asked for another review after fixing them.

(I've actually managed to figure out why ruff isn't sorting init.pyi: It's explicitly flagged as isort: skip at the top. I replied inline about this, but since it was on a resolved conversation, I'm putting it here to increase visibility. I do not intend, however, to resolve this issue right now, as it is orthogonal to this PR as long as I sort carefully manually.)

[freakboy3742]

One last tweak, but I think this is good enough to land as a base for actual platform implementations.

[johnzhou721]

Nice catch — thanks!

FWIW: I may choose to focus my free time during the next few days on a personal app so I may not be able to start backend implementations as soon as you might've hoped (and on top of that, I'm traveling). All that is to say, feel free to start on some backend implementations yourself if scaffolds are a big personal priority to you.