-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
Support GJS 1.72 Shell 42 #217
Conversation
@@ -3,7 +3,8 @@ const Self = ExtensionUtils.getCurrentExtension(); | |||
const Util = Self.imports.util; | |||
const OverviewControls = imports.ui.overviewControls; | |||
|
|||
const { SMALL_WORKSPACE_RATIO } = OverviewControls; | |||
//const { SMALL_WORKSPACE_RATIO } = OverviewControls; | |||
const SMALL_WORKSPACE_RATIO = 0.15; |
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.
Wait for Gnome Team to agree or disagree on opening this and other bits.
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.
Wait for Gnome Team to agree or disagree on opening this and other bits.
Did you open an issue or merge request at https://gitlab.gnome.org/GNOME/gnome-shell?
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.
With this and locally rebuilt shell (opening the above stuff) I now observe
Any ideas how to catch this NaN? :) |
Great work! I'd like to test this.
Would you mind sharing precisely what changes you made? |
Properly export WorkspaceAnimationController
…ate. Mark Shell 42 as targeted.
@@ -9,21 +9,23 @@ var ControlsManagerLayout = class { | |||
constructor() { | |||
this.originalLayout = null; | |||
this._overrideProperties = { | |||
_computeWorkspacesBoxForState(state, workAreaBox, searchHeight, dashHeight, thumbnailsHeight) { |
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 an incompatible change. @mzur, would you please open a shell-42 branch (or take this as it)?
Thanks for this! There's a problem with the pop-up not disappearing when I release CTRL+ALT. I need to either select the workspace with enter or kill the popup with ESC. |
Correction. It does not change workspaces for me at all. |
Some overrides for inaccessible Shell UI is not enough to cover everything (overview is broken), but it has to be disabled if you are running a patched shell. Edit: I've just pushed a couple of changes that should make the last commit compatible with a patched shell. |
Thanks for the explanation! How fast do you think the patched shell will be available? Is it worth it to invest time in building custom Debian packages? Can you please point me to the patches? |
Give that I saw noone in the shell PR, not soon. |
Thanks! |
I added a commit for drag and drop: zweif@60c0a0c |
First of all thanks for your work on porting this over to Gnome 42. I checked out this PR and most things work, besides having a bug in which when switching to any workspace without windows, the windows of a different workspace (couldn't determine which) would show up, as if they were on that workspace instead to begin with. I don't have much time and even less any knowledge on GJS but managed to pinpoint the issue via this journal error: [...]
abr 18 08:06:30 gnome-shell[10586]: Called enable_unredirect_for_display while unredirection is enabled.
abr 18 08:06:30 gnome-shell[10586]: JS ERROR: TypeError: Object 0x389ef8fc1988 is not a subclass of GObject_Object, it's a Object
_init@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:91:14
MonitorGroup@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:19:4
_prepareWorkspaceSwitch@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceAnimation.js:241:27
animateSwitch@resource:///org/gnome/shell/ui/workspaceAnimation.js:371:14
_switchWorkspace@resource:///org/gnome/shell/ui/windowManager.js:1632:34
actionMoveWorkspace@resource:///org/gnome/shell/ui/windowManager.js:1836:23
_showWorkspaceSwitcher@/home/emi/.local/share/gnome-shell/extensions/wsmatrix@martin.zurowietz.de/workspacePopup/workspaceManagerOverride.js:426:25
Caused by: Error: This JS object wrapper isn't wrapping a GObject. If this is a custom subclass, are you sure you chained up to the parent _init properly? I assumed the subsequent code was probably not being executed and wrapped the instantiation of try {
const stickyGroup = new WorkspaceGroup(null, monitor, movingWindow);
this.add_child(stickyGroup);
} catch (e) {} Let me know if there is anything I can do to help with the porting. |
@zweif This issue was already observed for GNOME Shell 40 (see #177). |
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.
As far as I can see, the only remaining issues are caused by the objects that can no longer be imported. I doubt that this issue will be processed quickly by the GNOME Shell core team. So if we want to release something for GNOME 42, we need to copy the core code. But we should do it in a way that is easily reversible once the objects can be imported again (probably for GNOME Shell 43).
_syncStacking() { | ||
const windowActors = global.get_window_actors().filter(w => | ||
this._shouldShowWindow(w.meta_window)); | ||
|
||
let lastRecord; | ||
|
||
for (const windowActor of windowActors) { | ||
const record = this._windowRecords.find(r => r.windowActor === windowActor); | ||
|
||
if (record && lastRecord) { | ||
this.set_child_above_sibling(record.clone, lastRecord ? lastRecord.clone : this._background); | ||
lastRecord = record; | ||
} | ||
} | ||
}, |
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.
@ettavolt Could you please explain why this is override necessary? The (record && lastRecord)
will never be true. Otherwise I see no difference to the upstream code.
@ebeem Could you please explain what the |
@mzur I think it overrides the |
I had a look and it seems to mostly work. Thanks everyone for fixing it. There's one regression though. In the previous version, the grid stayed on the screen as long as you kept CTRL+ALT pressed. Now, you either have to confirm the workspace selection with enter or set up a timeout for it to disappear. Any chance the old behavior can be restored? |
@ebeem Thanks! This is what it looks like for me if the override is disabled: I guess the override would fix the positioning of the workspaces on the second screen. But to be honest, I could live with the buggy behavior until the objects are made public again by GNOME Shell. Does anyone have objections? |
The override can be enabled again once the following issue is resolved and published in a new GNOME version: https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2266
gnome-shell-wsmatrix/[email protected]/workspacePopup/workspaceManagerOverride.js Line 451 in 28c191c
The return state here is null for some reason, I will try to debug it more, but this is either an upstream issue or there is some update that breaks this functionality |
I wasn't aware of this issue and it seems to match my observations, but drag and drop works for me with this change. |
Eagerly awaiting this... Anything that I can help with? |
This can continue in #218. I'll create a beta release from that branch. |
It didn't seem to have any effect. References #217 (comment)
Closes #212.
I'll go and ask Gnome team to open SecondaryMonitorDisplay along MonitorGroup and a couple of constants.
If they don't, we'll need to copy entire SecondaryMonitorDisplay and monkey-patch WorkspacesDisplay.