-
-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remember window position #1751
Remember window position #1751
Conversation
This reverts commit 5495618.
Thanks for this PR @lenemter , looking forward to this feature :-) According to the comment here: https://github.com/elementary/gala/pull/1751/files#diff-ba3fbdef1da3bc4b5454c2ad54c4a2751d0df4ed0dd64ee57c7c623bc161a570R63 Is there a hurdle tracking multiple windows of the same app - anyone have any ideas to solve this? |
I think if an application has two windows (e.g. Files opened with 2 windows), after the first (primary) window is being closed, gala should track the second window. I'll try to implement this now, but it's worth asking @danirabbit for her opinion. |
What about the use case of multiple windows of the same app and not closing any of them (or closing only some of them, still leaving multiple open) and user maybe logs out or restarts? I mean, say an app like Files or some third party say Firefox if I open 3 windows and move them around to my preferred positions, then close just 1 window (that should have its position saved) then while remaining 2 are still open, I logout/restart? Is it possible to handle this kind of scenario in this PR? Of course, yes, will leave it to core devs / UX team opinion. |
That's interesting. I'll look into this, shouldn't be too hard to implement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a code review but just wanted to mention I tried this PR and it appears to work well already in its current state.
I was going to ask about also saving apps' sizing information but looks like each app mostly does that for itself already. Tried Files, Music and Firefox.
It shouldn't be a issue. Even if user will launch 20 unique windows everyday for 5 years, it would be only 36k entries which I think isn't too much. |
@vjr I tried to implement your idea. Can you check if it is what you have in mind please? To test it you need to remove the old database: |
If someone wants to test this PR: There are two commits (versions) of this branch: b3203ac that implements #1751 (comment) and df0db86 that implements #1751 (comment) |
I think what @jeremypw had implemented in Files before was to only track the last closed window, but afaict this is working well to track all windows :) I'm sure he might have some opinions though? Nice work! Definitely super nice to have every window remembering position <3 |
Only the last closed window was remembered because only one window was restored. Saving and restoring all app windows was something I have considered but never got around to - I don't think there is a wishlist request for this? The state of each window (open tabs etc) would need to be saved so it would add to the complexity of the schema but is certainly doable if it is something that people want. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even for X11, this looks much nicer than forcing every app to implement it by itself 👍
I wanted to take a look at this PR to see how it handles multi-monitor setups. What macOS seems to do is that it remembers the last screen that a window was actively moved to. Disconnecting a secondary screen collects all windows on the remaining display, re-connecting it restores things as they were before.
I don't mean to ask for extra code in this PR to cover more edge cases, but do you think it makes sense to add a column for storing the preferred screen index(?) now to avoid having to migrate the DB later on?
(Although I guess one could always come up with more fields in the future, like was_fullscreen, workspace_index etc. so maybe YAGNI should win for now?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm inclined to get this in early so we can get lots of testing and potentially break the DB early if we have to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might not get to testing the PR until later in the weekend but I agree with @danirabbit comment to get this PR in early and work on any other issues , if discovered , as we go along.
@danirabbit I think currently window position is not saved before shutdown/sleep and doesn't save monitors correctly, but I'm too busy to implement it now. If this is ok for you we can merge this. |
Yeah we have several months to iterate! I'm sure we'll find other bugs too |
Late concern, and I'm not sure if I can make sense of it from the patch - what happens if you store a position and restore it when the setup changes such that the stored coords are out of bounds of the current setup? |
@orowith2os I don't know. As said it's tested very badly and was merged to get more feedback. |
I guess I'll be bug testing Elementary at some point in the future then - because this sounds like it could be Not Great. |
Fixes #297