Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix gnome 41.3 & 42 (pop & ubuntu 22.04) #101

Merged
merged 3 commits into from
Apr 10, 2022
Merged

Fix gnome 41.3 & 42 (pop & ubuntu 22.04) #101

merged 3 commits into from
Apr 10, 2022

Conversation

13r0ck
Copy link
Contributor

@13r0ck 13r0ck commented Mar 15, 2022 •
edited
Loading

This was referenced Mar 15, 2022
Merged
Closed
Closed
@Gronkdalonka
Copy link

Gronkdalonka commented Mar 30, 2022 •
edited
Loading

This fix does work on Gnome 41.4 (Debian bookworm)

@atd
Copy link

atd commented Mar 30, 2022

Thank you for this ❤️

@jm-duke
Copy link

jm-duke commented Mar 30, 2022

The fix is not complete, as it crashes the extension, when e.g. the lock screen activates:

gnome-shell[2341]: JS ERROR: Extension [email protected]: TypeError: WorkspacesView.SecondaryMonitorD isplay is undefined#012reset@

The reset() method still references WorkspacesView.SecondaryMonitorDisplay, which is no longer there. Removing this reference seems to work fine, no more crashes on my end.

@@ -26,7 +23,6 @@ function override() {
 
 function reset() {
     Util.overrideProto(WorkspacesView.WorkspacesView.prototype, global.vertical_overview.GSFunctions['WorkspacesView']);
-    Util.overrideProto(WorkspacesView.SecondaryMonitorDisplay.prototype, global.vertical_overview.GSFunctions['SecondaryMonitorDisplay']);
     if (global.vertical_overview.default_old_style_enabled) {
         Main.overview._overview._controls._workspacesDisplay.remove_style_class_name("vertical-overview");
     }

@13r0ck 13r0ck changed the title Fix for gnome 41.3 (pop & ubuntu 22.04) Fix gnome 41.3 & 42 (pop & ubuntu 22.04) Mar 31, 2022
@13r0ck
Copy link
Contributor Author

13r0ck commented Mar 31, 2022

This has been change to hopefully fix
#101 (comment)
& #98 (comment)

@thankjura
Copy link

thankjura commented Mar 31, 2022 •
edited
Loading

After apply this patch and modify metadata.js(for support gnome 42) I have bug:

Screenshot:

image

Update: revert overviewControls.js fixed this issue

Screenshot:

image

maxmorozoff
maxmorozoff approved these changes Apr 3, 2022
View reviewed changes
Copy link

@maxmorozoff maxmorozoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works. Thanks!

@colingibbs1 colingibbs1 mentioned this pull request Apr 4, 2022
Closed
RensAlthuis
RensAlthuis requested changes Apr 6, 2022
View reviewed changes
Copy link
Owner

@RensAlthuis RensAlthuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much! I do have some small requests, but then I'll merge and publisher as soon as I can.

workspacesView.js Outdated
@@ -15,7 +15,7 @@ const Util = Self.imports.util;

function override() {
global.vertical_overview.GSFunctions['WorkspacesView'] = Util.overrideProto(WorkspacesView.WorkspacesView.prototype, WorkspacesViewOverride);
log('You may see an error below,\nSecondaryMonitorDisplay is defined as const for some reason.\nSince I\'m overriding values in that const an error show might show up.\n Feel free to ignore it');
log('You may see an error below,\nSecondaryMonitorDisplay is defined as const for some reason.\nSecondaryMonitorDisplay = ' + WorkspacesView.SecondaryMonitorDisplay + '\nSince I\'m overriding values in that const an error show might show up.\n Feel free to ignore it');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this left over debugging? It's dumping the entire code for SecondaryMonitorDisplay into the log.

Copy link
Contributor Author

@13r0ck 13r0ck Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely enough this is required. I think it might be some GJS bug, but without printing WorkspacesView.SecondaryMonitorDisplay the use of it doesn't work in the next line saying that it cant be found. There might be a better way to accomplish this

workspacesView.js Outdated Show resolved Hide resolved
metadata.json Outdated Show resolved Hide resolved
RensAlthuis
RensAlthuis reviewed Apr 9, 2022
View reviewed changes
workspacesView.js Outdated Show resolved Hide resolved
@13r0ck @RensAlthuis
Update workspacesVi
a99a123 
Co-authored-by: RensAlthuis <[email protected]>
@RensAlthuis RensAlthuis merged commit 564dc75 into RensAlthuis:master Apr 10, 2022
@neclimdul
Copy link

manually installing this and getting my mental desktop management back in the groove today was the highlight of my morning. thanks for all the work y'all.

@axxapy
Copy link

axxapy commented Apr 16, 2022

@RensAlthuis This PR was merged but issue described in #101 (comment) is still there.
Only way to fix: git checkout 02bf3c4a646037ad65423a23d728108bcc9a892a -- overviewControls.js

@13r0ck 13r0ck deleted the gnome-41.3 branch April 27, 2022 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RensAlthuis RensAlthuis RensAlthuis requested changes

@maxmorozoff maxmorozoff maxmorozoff approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@13r0ck @Gronkdalonka @atd @jm-duke @thankjura @neclimdul @axxapy @RensAlthuis @maxmorozoff