slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
16.85k stars 556 forks source link

`StandardButton` implicit `clicked()` behavior #5693

Open dragonrider7225 opened 1 month ago

dragonrider7225 commented 1 month ago

What I expected to happen

Explicitly declaring callbacks related to StandardButtons (so that I can refer to them elsewhere in the dialog, e.g., with a FocusScope) does not break the buttons.

What actually happens

A StandardButton of ok kind only has an implicit clicked => { root.ok_clicked(); } callback if the root.ok_clicked() callback is not explicitly declared in the Dialog.

My minimal reproduction

Zipped

ogoffart commented 1 month ago

Thanks for the bug report.

Simplified, this illustrate the problem:

    // Functional: This Ok button works
    export component FunctionalDialog inherits Dialog {
        StandardButton { kind: ok; }
    }
    // "Broken: This Ok button doesn't work
    export component BrokenDialog inherits Dialog {
        callback ok_clicked();
        StandardButton { kind: ok; }
    }

That's because we only create the alias if the magic callback doesn't exist.

https://github.com/slint-ui/slint/blob/4afc3a2e84d422ced978368cb2654d7fc19b1615/internal/compiler/passes/lower_layout.rs#L538-L549

This code is responsible to create a alias such as callback ok_clicked <=> ok-btn.clicked;

Because we use a or_insert_with, we don't create it if it already exist. We could create one if it did exist, but we need to be careful that the signature match.

Unfortunately, if we are to implement that now, that would break your workaround as this will then be an infinite recursive call.

dragonrider7225 commented 1 month ago

I hadn't realized that callbacks could be aliases. Adding explicit aliases to the dialog definitions should be a better workaround than explicitly delegating the StandardButtons' clicked() callbacks to the root and setting the root's cancel handler in real_new().

But also, I'm not clear how making the StandardButtons' associated callbacks unconditionally being aliases would break my workaround. With the current behavior, when I delegate the StandardButtons' callbacks to the root, I also need to explicitly set the handler in Rust code for the callback that I'm delegating to, which would override the delegation to the root that could cause an infinite loop with the aliasing.