linuxmint / cinnamon

A Linux desktop featuring a traditional layout, built from modern technology and introducing brand new innovative features.
GNU General Public License v2.0
4.57k stars 744 forks source link

Applet settings window, widgets cannot update other widgets #12362

Open guillaume-mueller opened 2 months ago

guillaume-mueller commented 2 months ago

Distribution

Mint 22

Package version

6.2.9

Graphics hardware in use

No response

Frequency

Always

Bug description

I will refer the settings window of an Applet as UI.

  1. In the UI, any action initiated by any non-button widget cannot update another widget's value shown.

  2. While the inner value is well changed, the UI is like a new instance of the value and any subsequent change from the UI accounts for the shown value, while any other procedure in the Applet accounts for the inner value.

  3. While a button can update another widget's shown value, if a non-button widget has set a widget's value (incl. itself), the button won't be able to control the widget's shown value anymore.

Steps to reproduce

metadata.json:

{
    "uuid": "ui-cant-modify-itself@you",
    "name": "Bug demo: UI can't modify itself",
    "description": "-",
    "version": "1.0.0",
    "max-instances": "1"
}

settings_schema.json:

{
    "switch-master": {
        "type": "switch",
        "description": "Switch master",
        "default": false
    },
    "switch-slave": {
        "type": "switch",
        "description": "Switch slave, controlled by `Switch master` and `on_applet_clicked`",
        "default": false
    },
    "button-apply-master-to-slave": {
        "callback": "apply_false_to_slave",
        "type": "button",
        "description": "Apply `false` to slave"
    },
    "button-show-slave": {
        "callback": "show_slave_state",
        "type": "button",
        "description": "Show slave state"
    },
    "dull-switch-show-slave": {
        "type": "switch",
        "description": "Dull switch, executes `show_slave_state` on change",
        "default": false
    }
}

applet.js:

const Applet = imports.ui.applet;
const AppletSettings = imports.ui.settings.AppletSettings;
const Main = imports.ui.main;

class ThisApplet extends Applet.TextApplet {
    constructor(metadata, orientation, panel_height, instance_id) {
        super(orientation, panel_height, instance_id);

        this.set_applet_label("Open settings...");

        this.metadata = metadata;
        this.settings = new AppletSettings(this, metadata.uuid, instance_id);

        this.settings.bind('switch-master', "master", () => {
            this.slave = this.master;
            this.show_slave_state();
        });
        this.settings.bind('switch-slave', "slave");
        this.settings.bind('dull-switch-show-slave',  null, this.show_slave_state);
    }

    apply_false_to_slave() {
        this.slave = false;
        this.show_slave_state();
    }

    show_slave_state() {
        Main.notify("Info", "Switch slave is " + this.slave);
    }

    on_applet_clicked() {
        this.slave = !this.slave;
        this.show_slave_state();
    }
}

function main(metadata, orientation, panel_height, instance_id) {
    return new ThisApplet(metadata, orientation, panel_height, instance_id);
}

Expected behavior

The UI should always have its widgets' shown value up to date according to the inner value.

Additional information

No response

rcalixte commented 2 months ago

If I understand this issue correctly, I believe what you want instead is the bindProperty function instead of bind in tandem with the Settings.BindingDirection.BIDIRECTIONAL attribute:

        this.settings.bindProperty(Settings.BindingDirection.BIDIRECTIONAL,
                                   'switch-master', 'master',
                                   this.switch_master, null);
        this.settings.bindProperty(Settings.BindingDirection.BIDIRECTIONAL,
                                   'switch-slave', 'slave',
                                   null, null);
        this.settings.bindProperty(Settings.BindingDirection.BIDIRECTIONAL,
                                   'dull-switch-show-slave', null,
                                   this.show_slave_state, null);

        switch_master() {
            this.slave = this.master;
            this.show_slave_state();
        }

Some variant of this code should work for you if I've understood this correctly.

guillaume-mueller commented 2 months ago

Are you able to make it work ? I don't.

Actually I tried a lot of things including this one before reporting the issue and it always behaved the same.

Also from Devhelp about bindProperty:

[…] This function is deprecaed and is now only a wrapper around bind for backward compatibility. Please use bind instead.

rcalixte commented 2 months ago

Are you able to make it work ? I don't.

I believe this is working on other applets currently. My assumption is that I properly understood your issue but maybe not. Would you be able to generate a video of the issue?

Also from Devhelp about bindProperty:

[…] This function is deprecaed and is now only a wrapper around bind for backward compatibility. Please use bind instead.

Well, that's unfortunate...

guillaume-mueller commented 2 months ago

https://github.com/user-attachments/assets/fd0c2c74-a24b-400a-beed-f052bf7c4d96

  1. In the UI, any action initiated by any non-button widget cannot update another widget's value shown.
  1. open the settings window with slave set to false,
  2. change master to true => slave is incorrectly not updated,
  3. close the settings window and open a new one => slave on UI is correctly updated,
  4. change master, finish on true,
  5. press button Apply `false` to slave => slave is correctly updated. 5(sub). I cannot reproduce 5. consistently, but without doing 4. it appears to always correctly update.

https://github.com/user-attachments/assets/b0fa5294-1ed1-4a95-b995-5b2e6d53eee5

  1. While the inner value is well changed, the UI is like a new instance of the value and any subsequent change from the UI accounts for the shown value, while any other procedure in the Applet accounts for the inner value.
  1. show slave state with button Show slave state => correct,
  2. show slave state with dull switch => correct,
  3. change master,
  4. show slave state with button Show slave state => correct,
  5. show slave state with dull switch => incorrect,
  6. Bonus: close the settings window and reopen it => having used the dull switch has weirdly made the incorrectly not updated UI slave value to be incorrectly reapplied.

https://github.com/user-attachments/assets/5cbbfe26-b074-402e-b24e-9c12de8538c1

  1. While a button can update another widget's shown value, if a non-button widget has set a widget's value (incl. itself), the button won't be able to control the widget's shown value anymore.
  1. open the settings window with slave on true,
  2. press button Apply `false` to slave => slave is correctly updated,
  3. change slave, finish on true
  4. press button Apply false to slave => slave is incorrectly not updated 4sub. it hasn't been 100% consistent but very mostly had this behavior.
guillaume-mueller commented 2 months ago

Are you able to make it work ? I don't.

I believe this is working on other applets currently. My assumption is that I properly understood your issue but maybe not. Would you be able to generate a video of the issue?

Also from Devhelp about bindProperty:

[…] This function is deprecaed and is now only a wrapper around bind for backward compatibility. Please use bind instead.

Well, that's unfortunate...

Or maybe it is not unfortunate as the bind method may already set as bidirectionnal but the bug I'm pointing is what prevent it to work.

rcalixte commented 2 months ago

Or maybe it is not unfortunate as the bind method may already set as bidirectionnal but the bug I'm pointing is what prevent it to work.

I'm honestly not sure. What I do know is that I've implemented these features by using the bidirectional bindings. If the documented method isn't working as intended though, it would be great to fix it properly.