-
Notifications
You must be signed in to change notification settings - Fork 171
Compatibility with Fullscreen Avoider #707
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
Draft
Lythenas
wants to merge
27
commits into
paperwm:develop
Choose a base branch
from
Lythenas:fullscreen-avoider-compat
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+172
−26
Draft
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
bcbad9f
Patch monitor.inFullscreen to check PaperWM spaces
Lythenas 2143e0c
Stop hiding topbar
Lythenas 6b5a125
Move patch to patches.js
Lythenas 1e83182
Hide our topbar when the real one is moved onto it
Lythenas 41bbb92
Show correct info when panel is not on primary monitor
Lythenas 2e0f9cc
Don't shrink workarea if panel was moved to secondary monitor
Lythenas 862c0e6
Expose paperwm in global variable
Lythenas a40d6d1
Fix null check
Lythenas 03f7196
Check position of monitor instead of backdrop
Lythenas 33acbd9
Add undefined check to patch
Lythenas 45d14fc
Always update topbar workspace indicator during switching
Lythenas 270b2c4
Run layout after workspace switch animation
Lythenas dde175b
Fix topbar when swapping workspaces
Lythenas 718af77
Don't change topbar style when updating window position indicator
Lythenas ee87d11
Set topbar elements visible by default
Lythenas 24cc09b
Hide focus mode icon if topbar is hidden on ws
Lythenas 333b1ce
Add helper Topbar.isOnMonitor
Lythenas dfd39d6
Only update topbar ws name if on the same monitor
Lythenas b1e9391
Fix detection when window starts in fullscreen
Lythenas 4d829a8
Queue layout after panelBox position changed
Lythenas 6d2ae74
Merge remote-tracking branch 'upstream/develop' into fullscreen-avoid…
Lythenas d780540
Use correct space in fixTopBar
Lythenas a0263a1
Re-layout when workarea changes
Lythenas 0ed9646
Merge remote-tracking branch 'upstream/develop' into fullscreen-avoid…
Lythenas 7c7f2c9
Fix jumpyness when entering fullscreen
Lythenas 9d8799b
Detect floating and scratch windows as fullscreen
Lythenas d67b64e
Merge remote-tracking branch 'upstream/develop' into fullscreen-avoid…
Lythenas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
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.
@Lythenas, that this type of patching needs to be done in patches.js and using the patterns there, otherwise this wouldn't pass EGO review) - see examples there for patching prototypes (which will then be automatically reverted on PaperWM disable).
But in principal I can see what you've done here.
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.
All good though - we can do that after.
Uh oh!
There was an error while loading. Please reload this page.
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.
Didn't know about the
patches.jsfile. Will move it there.But I think we can't use the pattern there, sincemonitor.prototypeis undefined. Also I am not overwriting a normal function or property. I am setting a getter function for a property. And since that does not exist not sure how to restore it. I guess we would just safe the undefined.Edit: I think the code in
patchesworks directly on classes, right? So I don't think we can use it sincemonitorare just plain JS objects as far as I can tell.The prototype seems to apply to all monitor objects but no other objects for some reason. I also don't know what happens when you plug in or unplag a monitor afterwards or change the monitor configuration. It is possible that the prototype will be lost when the objects change.
Maybe there is a better way to patch this but that is what I found works.
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.
Also note that fullscreen avoider does some interesting stuff for patching by calling
toStringon the function and string replacing some things: https://github.com/Noobsai/fullscreen-avoider/blob/master/src/extension.js#L91Uh oh!
There was an error while loading. Please reload this page.
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 see, given what we're doing in this case, we can add it into
Utils.jsupgradeGnomeMonitors:https://github.com/paperwm/PaperWM/blob/release/utils.js#L526-L536
This is nice since it's a dbus service hook that will get called whenever monitors change (e.g. if we add new monitors and remove monitors). We use this to upgrade monitor objects to add connector information (e.g. HDMI-1, eDP-1 etc.).
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.
anyway, can leave as is for the time being (since it's working fine) and can focus on the other issues (that you mentioned).
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.
No,
Monitoris indeed a class - it's here:https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/ui/layout.js?ref_type=heads#L158-L171
You're essentially overriding the prototype class getter
get inFullscreen().Interestingly, Gnome haven't exported this so that's the main reason why we can't use it directly in patches. You're approach of getting a concrete implementation of one and overriding the prototype (which effects the Monitor class) will work though.
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.
Ah ok. I thought is is not a class because it only shows up as an
[object Object]in looking glass. But.constructor.nameindeed returnsMonitor.