Add support for Gnome 43.#884
Conversation
|
Prettier does fail can you fix the formating? |
|
The Prettier fails because it cannot check out my branch. Its log shows that it doesn’t even try to pull from my fork, so I don’t think I can actually fix anything. Edit after merge: Prettier actually found issues, but the CI job didn’t. |
| const mainChild = actor.get_children()[0] as Clutter.Actor | undefined; | ||
| if (mainChild !== undefined) { | ||
| if (mainChild instanceof panel.AggregateMenu) { | ||
| if (mainChild instanceof (compareVersions(gnomeVersionNumber, parseVersion('43.0')) >= 0 ? panel.QuickSettings : panel.AggregateMenu)) { |
There was a problem hiding this comment.
We want to check for gnome 43 and greater not only 43 you can use gnomeVersionGreaterOrEqualTo
There was a problem hiding this comment.
It does check for equal or greater to.
There was a problem hiding this comment.
Also, if I read the code correctly, gnomeVersionGreaterOrEqualTo requires using an instance of a TypeWithGnomeVersion, which I didn’t define. Maybe it is possible to do that, but I don’t have the knowledge of TypeScript to determine that or, if so, how it’s done.
There was a problem hiding this comment.
Personally I wouldn't have used version check here, I would've just ored the two conditions, since both instances won't ever happen at the same time anyway. But I guess this is a little more explicit in intent. I like simplicity tho.
Edit: and for explicitness, if necessary, a comment would suffice.
Edit2: I think the version util needs a simpler method for easy cases like this one. This check is ugly.
Edit3: What I just said in edit2 doesn't affect this PR tho, I guess I'm just venting, sorry for that.
|
I tested this PR on 42 for regressions, no problems here. lgtm |
4 participants
In Gnome 43, the system status menu was replaced by a new one with a different type. The type is used only once in material shell. This PR adds a version check around the usage to choose the correct type based on the gnome-shell version.
I have pretty much no experience with TypeScript yet, so there may be a better way of performing this check, maybe by using the
IntroducedInGnomeandRemovedInGnometypes.