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.

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

refactor: GNOME 46 port #1704

Merged
merged 1 commit into from
Mar 25, 2024
Merged

Conversation

carlwgeorge
Copy link
Contributor

It looks like .add() and .add_actor() were removed in GNOME 46. Replacing these with .add_child() seems to work. With these changes I'm able to successfully install and use the extension on GNOME 46 RC.

oae/gnome-shell-pano#259
https://gitlab.gnome.org/GNOME/gnome-shell/-/commit/350cd296fa90adb3d18f9b375b5531077cff825a

It looks like .add() and .add_actor() were removed in GNOME 46.
Replacing these with .add_child() seems to work.  With these changes I'm
able to successfully install and use the extension on GNOME 46 RC.

oae/gnome-shell-pano#259
https://gitlab.gnome.org/GNOME/gnome-shell/-/commit/350cd296fa90adb3d18f9b375b5531077cff825a
@ghostdevv
Copy link
Contributor

gjs gnome 46 upgrade notes

Is this backwards compatible to gnome 45?

@carlwgeorge
Copy link
Contributor Author

Yes, as far as I can tell. .add_child() is present in GNOME 45.

@RealSuperRyn
Copy link

These might have to be modified, but I "ported" (maybe only partially, idk tbh) an existing installation from the AUR package to gnome 46 by going in the extensions directory and using the following commands:
(sudo is just because it is not in a directory i had write access to)

sudo sed -i 's/add_actor/add_child/g' context.js panel_settings.js search.js stack.js
find . | sudo sed -ri -e 's/.add(/.add_child(/g' /.js

(This was also after applying all of the existing patches this pr has as of posting)

@jackpot51 jackpot51 requested a review from a team March 24, 2024 17:34
@spladug
Copy link
Contributor

spladug commented Mar 24, 2024

There's another add() call that needs to be updated on L186 of src/stack.ts. Stacking is broken without it.

Edit to add: and two more in src/search.ts: L344 and L367. Without these, the launcher shows no entries. Note that even with these fixes the launcher is visually a bit off; the items are now oddly centered.

@fabifont
Copy link

fabifont commented Mar 25, 2024

Hi, is this PR ready to be merged? I am currently testing it on my PC and it works without problems.

@leviport
Copy link
Member

Hi, is this PR ready to be merged? I am currently testing it on my PC and it works without problems.

Not yet. The QA team needs to make sure no regressions are introduced in Pop 22.04.

@jackpot51
Copy link
Member

@leviport this won't work in Pop 22.04, which is why it is targeting the master_mantic branch. It should work on GNOME 45 and 46.

Copy link
Member

@leviport leviport left a comment

Choose a reason for hiding this comment

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

Ah sorry about that, I missed that it wasn't going into master. Since it's not touching Jammy, I see no reason to test for regressions.

@jackpot51
Copy link
Member

@carlwgeorge do you want to address @spladug's comment before merging?

@spladug
Copy link
Contributor

spladug commented Mar 25, 2024

I can make a follow-up PR if that's easier for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants