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

Conditional item gets re-instantiated even if the result is the same #3953

Open
bjorn opened this issue Nov 17, 2023 · 3 comments
Open

Conditional item gets re-instantiated even if the result is the same #3953

bjorn opened this issue Nov 17, 2023 · 3 comments
Labels
a:models&views The implementation of the `for` and ListView (mO,bF) bug Something isn't working

Comments

@bjorn
Copy link
Contributor

bjorn commented Nov 17, 2023

Platform: Linux / winit / FemtoVG / X11, Language: Rust

Consider the following code:

if (Facade.portfolio.file-name != ""): MainContent {}

When I call facade.set_portfolio, the MainContent appears to be always re-instantiated (judging by the loss of its state), even if the result of the condition stays true. This is undesirable.

I'm working it around in my application by instead doing:

MainContent {
    visible: Facade.portfolio.file-name != "";
}

But this of course keeps the instance around, even when it is not needed.

bjorn added a commit to bjorn/raccoin that referenced this issue Nov 17, 2023
It seems that due to re-evaluation of the binding, the MainContent got
re-instantiated, causing it to switch back to the Portfolio page each
time the Facade.portfolio was set.

See slint-ui/slint#3953
@ogoffart
Copy link
Member

The thing is that we have to reset the whole thing if the model is dirty in

if model.is_dirty() {
*self.data().inner.borrow_mut() = RepeaterInner::default();

But we could actually check if the model has changed before doing that.

Now the problem is how to compare two model. Because the generated expression in rust is sp::ModelRc::new(#model as bool), so it's always a different model. So we need to find a trick to make it work. Maybe instead of using a ModelRc for bool model, we could use something else.

@ogoffart ogoffart added bug Something isn't working a:models&views The implementation of the `for` and ListView (mO,bF) labels Nov 17, 2023
@bjorn
Copy link
Contributor Author

bjorn commented Nov 17, 2023

So we need to find a trick to make it work. Maybe instead of using a ModelRc for bool model, we could use something else.

Or maybe it is possible to do a special case check for comparing bool models? Using as_any / downcast_ref, maybe?

ogoffart added a commit that referenced this issue Dec 28, 2023
If resizing makes the value of is-resizable dirty, we would hit issue #3953
and the handle would be re-created and the drag operation aborted
ogoffart added a commit that referenced this issue Jan 2, 2024
If resizing makes the value of is-resizable dirty, we would hit issue #3953
and the handle would be re-created and the drag operation aborted
@tp971
Copy link

tp971 commented May 25, 2024

I just came across this issue after updating an older private project; this seems to be a recession from 1.2.2 to 1.3.0. In my case, I had a conditional button in a video-player-like application, which is almost impossible to click because the player state is updated 60 times a second:

if (!state.playing): Button {
    icon: @image-url("icons/play-fill.svg");
    enabled: state.ready && !state.fastforward;
    clicked => {
        play();
        focus.focus();
    }
}
if (state.playing): Button {
    icon: @image-url("icons/pause-fill.svg");
    enabled: !state.fastforward;
    clicked => {
        pause();
        focus.focus();
    }
}

I could workaround this issue by "pulling the ifs inside":

Button {
    icon: !state.playing ? @image-url("icons/play-fill.svg") : @image-url("icons/pause-fill.svg");
    enabled: state.ready && !state.fastforward;
    clicked => {
        if (!state.playing) {
            play();
            focus.focus();
        } else {
            pause();
            focus.focus();
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:models&views The implementation of the `for` and ListView (mO,bF) bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants