Skip to content

Conversation

@sqrvrt
Copy link
Contributor

@sqrvrt sqrvrt commented Oct 16, 2025

This competes with #8096.

Make a container to wrap the widget while detached that mirrors functionality of SubWindow to a certain degree. Eliminates the need for Detachable{Widget,Window} classes since we no longer need to rely on inheritance.

Questionable design decisions:

  • Instead of initializing DetachedWindow once and hiding it, it's constructed each time a window is detached and destroyed once the window is attached back. This makes them slightly easier to manage.
  • DetachedWindow doesn't store SubWindow pointer in a variable, instead doing dynamic_cast<SubWindow&><*parentWidget()>, making it dependant on keeping the parent the same.
  • Despite DetachedWindow being basically dependant on SubWindow to exist, it's still kept as a pointer rather than being attached to the class. This is mostly to keep the syntax consistent.

If any of these are undesirable, I can try to change them.

TODO:

  • Remove Detachable* classes
  • [needs testing] Sync title and icon to widget
  • [needs testing] Prevent widgets with size restrictions from maximizing (mirror SubWindow functionality)
  • Prevent detached windows attaching out-of-bounds Subject for a different PR

Wrap widget being detached into a container to handle close events
correctly without relying on inheritance. Makes it possible to add
additional widgets (for example separate button to attach the window back)
These are no longer needed, as the functionality is implemented in
DetachedWindow and does not require inheritance. This is a mechanical
check to remove files, rudimentary functionality may still be present.
@sqrvrt sqrvrt marked this pull request as ready for review October 17, 2025 10:29
@messmerd messmerd requested a review from bratpeki October 17, 2025 16:07
@bratpeki bratpeki self-assigned this Oct 17, 2025
@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 18, 2025

Probably best to wait before merging, I might have a simpler solution

edit: added competing PR to the post.

@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 19, 2025

Closing this in favor of #8096. Can reopen if there's any reason for it.

@sqrvrt sqrvrt closed this Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants