Skip to content

Conversation

@sqrvrt
Copy link
Contributor

@sqrvrt sqrvrt commented Oct 14, 2025

Rationale:

  • This is way more intuitive, and more discoverable than the previous method of having to "open" the window while it's detached.
  • This solves the issue of things attaching themselves when simply raised. (Example: opening a different segment in piano roll would cause it to attach back since show() is called)

Drawbacks:

  • Slightly more complex in implementation
  • Detachable widgets now require their parent to be SubWindow specifically, in current implementation could throw a segfault if not used carefully. Probably can be fixed, but I don't know the preferable method. Additionally, it doesn't seem that DetachableWidget has any uses outside the SubWindow, so it could be narrowed down to a compile-time check (unsure about plugins though)

Additional considerations:

  • Action could be invoked with minimize button, with close just closing the window. I'd argue in that case close acts like a minimize button since no windows are actually ever closed, and the minimize action feels more arbitrary. Additionally, many linux WMs (notably Gnome) don't even support minimizing the windows in a traditional way.
  • Attach/detach activating on keybinds. Most of LMMS can function to a certain degree without keybinds, so introducing an arbitrary one without an alternative would limit its uses in touch-only mode. Additionally a keybind manager would be preferrable, and additional measures need to be taken for people to be aware the feature even exists.
  • Client-side decorations. This requires rewriting most of the SubWindow changes, and outright failed to work with my initial attempts. If anyone has better luck, they're welcome. Worthy of note, additional actions would be required to make floating windows stand out against attached ones, plus this wouldn't fit desktop theming.

@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 14, 2025

Again, this probably shouldn't be directly merged as of right now since assuming the parent widget is a SubWindow has very high segfault potential. I'd be more than happy to implement any of suggested solutions

However even this build is still useful as a proof-of-concept to compare against current behavior.

no longer relevant

@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 14, 2025

Done.

It seems many secondary widgets such as ITW and similar do get closed instead of attaching though, perhaps it's a matter of simply making them inherit the mentioned DetachableWidget (&-Window).

I do believe this would better be done by having a transparent "event widget" acting as middle man between these two, so that the detached component stays in a container (we currently expose it directly and need inheritance to have shared functionality). This could also act as a starting ground for CSD if we ever decide we want them. However I think that goes out of scope of this particular PR.

Note: most CI builds fail because the workflow files are not updated from master.

@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 14, 2025

lmms.zip
Since builds are failing, here's a minimal test binary I used. For anything else either compile it yourself or rebase feature/detach-window on a newer commit.

@sqrvrt sqrvrt force-pushed the detachwindow/attach-on-close branch from 13379db to fab4127 Compare October 15, 2025 01:36
@messmerd
Copy link
Member

I'll merge, then resolve the merge conflicts in the feature/detach-window branch.

@messmerd messmerd merged commit 937c8e1 into LMMS:feature/detach-window Oct 16, 2025
6 of 9 checks passed
@messmerd
Copy link
Member

Looks like instrument windows do not reattach after they've been detached.

And due to #7595, if I detach a window, move it to my 2nd monitor, then click the close button to reattach it, the window is harder to find because no scrollbars are present in the workspace anymore. Users who don't know they can drag the workspace to navigate around will probably be unable to find the window at all.

@sqrvrt
Copy link
Contributor Author

sqrvrt commented Oct 16, 2025

@messmerd

Looks like instrument windows do not reattach after they've been detached.

I'm working on rewriting Detachable{Widget,Window} to be a wrapper rather than an ancestor, so that it works with any arbitrary widget without having to modify its source. It will also enable us to put additional elements in case we want CSD or at least a GUI attach button. Though it seems that to reuse SubWindow elements we'd have to duplicate the code which isn't great.

And due to [pull], if I detach a window, move it to my 2nd monitor, then click the close button to reattach it, the window is harder to find because no scrollbars are present in the workspace anymore. Users who don't know they can drag the workspace to navigate around will probably be unable to find the window at all.

I don't have a multi-monitor setup, so I don't really know how that works. If I understand correctly, X11 multimonitor setups where windows are movable between them are pretty much just a cut view of a big X11 canvas, so realistically I don't see any way to check if a window is on a different monitor and whatnot. And I also don't know how Windows part of this works, with all its different modes.

Maybe I should just check for top-left corner being out of bounds, and if it is just don't move the SubWindow? I.e. out of bounds that would work the way it currently works in wayland-native mode.

@sqrvrt sqrvrt deleted the detachwindow/attach-on-close branch October 16, 2025 11:56
@messmerd
Copy link
Member

I don't have a multi-monitor setup, so I don't really know how that works.

You should be able to achieve the same effect by letting the main LMMS window occupy half of your screen, moving a detached window to the other half of your screen, then closing it.

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