Review of "GameMode Shell Extension" version 18

Details Page Preview

GameMode Status Indicator for GNOME Shell!

Extension Homepage
https://github.com/trsnaqe/gamemode-shell-extension

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 1

Shexli found 1 issue that may need reviewer attention.

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • extension.js:90
            this._client.connect(
              "count-changed",
              this._updateClientList.bind(this)
            )
  • extension.js:94
            this._client.connect(
              "game-registered",
              this._updateClientList.bind(this)
            )
  • extension.js:98
            this._client.connect(
              "game-unregistered",
              this._updateClientList.bind(this)
            )
  • extension.js:102
            this._client.connect(
              "state-changed",
              this._handleStatusChange.bind(this)
            )
  • extension.js:142
          this._doNotDisturbToggle.connect("toggled", (item, value) => {
            this._settings.set_boolean("enable-do-not-disturb", value);
          })
  • extension.js:158
          this._iconVisibilityToggle.connect("toggled", (item, value) => {
            this._settings.set_boolean("show-icon-only-when-active", value);
          })
  • extension.js:131
          this._notificationCloseToggle.connect("toggled", (item, value) => {
            this._settings.set_boolean("show-close-notification", value);
          })
  • extension.js:122
          this._notificationLaunchToggle.connect("toggled", (item, value) => {
            this._settings.set_boolean("show-launch-notification", value);
          })
  • extension.js:297
          this._settings.connect("changed::active-color", () => {
            if (this._client && this._client.current_state) {
              const activeColor = this._settings.get_string("active-color");
              this._icon.set_style('color: ' + activeColor + ';');
            }
          })
  • extension.js:311
          this._settings.connect("changed::enable-do-not-disturb", () => {
            const enableDoNotDisturb = this._settings.get_boolean("enable-do-not-disturb");
            this._doNotDisturbToggle.setToggleState(enableDoNotDisturb);
    
            if (this._client && this._client.current_state) {
              if 
  • extension.js:304
          this._settings.connect("changed::inactive-color", () => {
            if (this._client && !this._client.current_state) {
              const inactiveColor = this._settings.get_string("inactive-color");
              this._icon.set_style('color: ' + inactiveColor + ';');
            }
          })
  • extension.js:290
          this._settings.connect("changed::show-close-notification", () => {
            const showCloseNotification = this._settings.get_boolean(
              "show-close-notification"
            );
            this._notificationCloseToggle.setToggleState(showCloseNotification);
          })
  • extension.js:275
          this._settings.connect("changed::show-icon-only-when-active", () => {
            const iconVisibilitySetting = this._settings.get_boolean(
              "show-icon-only-when-active"
            );
            this._iconVisibilityToggle.setToggleState(iconVisibilitySetting);
            this.visible = !iconVisibi
  • extension.js:283
          this._settings.connect("changed::show-launch-notification", () => {
            const showLaunchNotification = this._settings.get_boolean(
              "show-launch-notification"
            );
            this._notificationLaunchToggle.setToggleState(showLaunchNotification);
          })

All Versions

Version Status
20 Active
19 Rejected
18 Active
17 Active
16 Active
15 Active
14 Active
13 Active
12 Active
11 Active
10 Active
9 Inactive
8 Inactive
7 Active
6 Inactive
5 Inactive
4 Inactive
3 Inactive
2 Rejected
1 Rejected

Previous Reviews on this Version

fmuellner active
It would be good to address the shexli warnings. Actors that are destroyed when the extension is disabled won't emit any signals anymore, but that does not apply to objects like this._settings and this._clients. You may want to use "connectObject()" there, to automatically disconnect the signals when the "owning" actor is destroyed.