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

Address deprecation warnings #56

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alessandrodolci
Copy link
Contributor

Hello, I went on to fix the problems reported in #54, here is a brief summary:

ByteArray string representation

Calls to ByteArray toString method have been made explicit on Uint8Array objects returned by GLib functions. Compatibility with older Gnome Shell versions has been kept by checking the object type and invoking the method in the correct way.

Actor property on UI classes

Access to actor property belonging to UI classes instances has been replaced by direct access to the instances. This has been done for both DockerMenu as well as DockerSubMenuMenuItem, whose superclasses have now become Clutter actors themselves.

Actually, the warning appeared only for the first of the two classes, but digging inside the library source code for UI elements I found out that the case must be the same for both of the two base classes (namely, PanelMenu.Button and PopupMenu.PopupSubMenuMenuItem), as this exists on the second one, explaining the absence of the warning message:

get actor() {
        /* This is kept for compatibility with current implementation, and we
           don't want to warn here yet since PopupMenu depends on this */
        return this;
    }

I hope you'll find this clear enough and acceptable as well, as always I'll be happy to clarify a bit more or investigate further if needed.

Thank you!

@@ -41,8 +41,8 @@ var DockerMenu = class DockerMenu extends PanelMenu.Button {
const dockerIcon = new St.Icon({ gicon: gicon, icon_size: "24" });

hbox.add_child(dockerIcon);
this.actor.add_child(hbox);
this.actor.connect("button_press_event", this._refreshMenu.bind(this));
this.add_child(hbox);
Copy link
Owner

Choose a reason for hiding this comment

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

Just tested in with:

  • Ubuntu 18.04
  • Gnome Shell 3.28.4

and the extension does not start because of error this.add_child is not a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh no, my bad :(
I'm very sorry but I have not been able to test on older OS/Gnome versions. I think that the moment has come to spin up a VM running Ubuntu 18.04 (I think it's the main "legacy" version that is worth supporting) for testing purposes.

However, this shouldn't be a too much of a problem, I'll try to add some sort of check on the class of the objects before invoking those methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I came out with a solution that involves checking the type of the instance at runtime, I hope you'll find it acceptable.
The alternative could be to ignore the warnings for now and deal with them when it will become mandatory.

I took the chance to bring up a VM with 18.04 and gs 3.28.4, which allowed me to test for compatibility issues, no problems detected.

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.

2 participants