ShellClients: Better separation of concern #2575
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leolost2605
changed the title
leolost2605
force-pushed
the
leolost/separate-shell-window
branch
3 times, most recently
from
1afaa18 to
00da2f0
Compare
This was referenced Oct 15, 2025
lenemter
reviewed
Oct 15, 2025
| x = monitor_geom.x + (monitor_geom.width - window_rect.width) / 2; | ||
| y = monitor_geom.y; | ||
| break; | ||
| var monitor_rect = window.display.get_monitor_geometry (window.display.get_primary_monitor ()); |
Member
There was a problem hiding this comment.
Shouldn't we use window monitor here?
Suggested change
| var monitor_rect = window.display.get_monitor_geometry (window.display.get_primary_monitor ()); | |
| var monitor_rect = window.display.get_monitor_geometry (window.get_monitor ()); |
Member
Author
There was a problem hiding this comment.
For panels we definitely want the primary monitor so that they actually are on it. For centered windows idk but ig it's better if they stay on the monitor they are started on. Therefore I changed it to not give the monitor geom as argument so that the implementations are responsible for choosing the correct one (currently primary for panels and the windows' for centered).
Move things that only apply to PanelWindows there. Also rely on subclasses to provide a GestureTarget for property updates instead of trying to handle all cases. This cleans up a lot of code that was necessary to handle the stuff needed by all subclass. This makes sure we only update things when actually needed. For example ExtendedBehaviorWindows never need to update their target. This will also provide more flexibility in the future if we want to introduce other specialized windows. This also allows to change more stuff in subclasses without affecting other subclasses.
Instead of an enum based on which the position is calculated, introduce an abstract method that should be overridden by subclasses for position calculation. This introduces more flexibility in the future while reducing code complexity.
leolost2605
force-pushed
the
leolost/separate-shell-window
branch
from
00da2f0 to
9913b3c
Compare
lenemter
approved these changes
Oct 15, 2025
Member
|
Oops, I forgot to rebase. Sorry! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of trying to handle everything a subclass could want in PositionedWindow/ShellWindow only provide a framework that all subclasses need and let them handle their specificity.
This includes having subclasses provide a gesture target for how they want to handle hiding instead of setting it based on position.
This also includes removing the position enum and introducing an abstract method for position calculation.
All in all this reduces code complexity and makes it easier to change things for one subclass without risking to introduce regressions for others.
This also provides more flexibility in the future. Most notably I wanted to do this before finishing #1974 because there I wanted to introduce a ExtendedBehavior subclass anyways. Other things include monitor labels and maybe even notifications which will all be easier to handle this way.
See the individual commits for easier review