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

property bindings are lost after connecting qml list models to the .Net models using "model: Net.toListModel()" #166

Open
MosiQt opened this issue Sep 19, 2019 · 22 comments

Comments

@MosiQt
Copy link

MosiQt commented Sep 19, 2019

ListView in QML needs a ListModel to populate data. Also you need to define a delegate to define how data inside a model should be displayed in the view. (Link).

After working more than 10 years with Qt, I've found this as one of the best features of Qt/Qml. Most important, you can use property binding of model element in the delegates. It means by changing the model data, all the delegates will update theme-self. No extra refreshes are needed!

Now it seems that these property bindings are no longer working in qml.net. It's a huge drawback if there is no solution considered. I've prepared a simple example (The sample is attached):

This is a ListView in Qml with a simple delegate.
2

The data are comming from .Net model:
2

Whenever user clicks on the each row, I try to change title of that row:
3
4

The title is changed successfully yet we can't see the change in the view!
5
(the title is remained fixed though it's changed in the model)

Therefore it seems that the .Net models are making all property bindings gone unless there is a solution I'm not aware of that :) I'll appreciate if anyone can help me.
Example.zip

@pauldotknopf
Copy link
Member

You are declaring the NotifySignal, but you aren't activating it when the value changes.

See this: https://github.com/qmlnet/qmlnet/blob/develop/src/net/Qml.Net/Signals.cs

@MosiQt
Copy link
Author

MosiQt commented Sep 22, 2019

You are declaring the NotifySignal, but you aren't activating it when the value changes.

See this: https://github.com/qmlnet/qmlnet/blob/develop/src/net/Qml.Net/Signals.cs

Thanks Paul. Now I've added ActiveSignal call but not working yet. Let me know if I'm doing something wrong:
2019-09-22 11_38_25-ProjectViewModel cs - SkeletonApp - Visual Studio Code  Administrator

Should I register the "titleChanged" signal or connect it to somewhere?!

@MosiQt
Copy link
Author

MosiQt commented Sep 22, 2019

You are declaring the NotifySignal, but you aren't activating it when the value changes.
See this: https://github.com/qmlnet/qmlnet/blob/develop/src/net/Qml.Net/Signals.cs

Thanks Paul. Now I've added ActiveSignal call but not working yet. Let me know if I'm doing something wrong:
2019-09-22 11_38_25-ProjectViewModel cs - SkeletonApp - Visual Studio Code Administrator

Should I register the "titleChanged" signal or connect it to somewhere?!

Paul I've found the issue. The property binding is not working in delegates in qml. However it works when applying to ordinary qml items like Text. How can I make it works in delegates?

@pauldotknopf
Copy link
Member

Can you give me a minimal repo, maybe a repository somewhere I can clone?

@MosiQt
Copy link
Author

MosiQt commented Sep 22, 2019

Can you give me a minimal repo, maybe a repository somewhere I can clone?

Check out this: https://github.com/MosiQt/TestQmlDotNet.git

Please check Views/MainFeed.qml (line: 33). Whenever user clicks on a list item, its title is changed (in C#) but the change is not affected in the list (delegate)

@MosiQt
Copy link
Author

MosiQt commented Sep 23, 2019

@pauldotknopf

Also I've found 2 more clues that may be helpful (I've update the repo: https://github.com/MosiQt/TestQmlDotNet.git):

1- In the delegate (qml), always the last item is updated! So weird.
Look at this part of my repo code (FeedItemControl.qml) which is the list delegate definition:

Label {
	  id: titleLabel
	  text: modelData.title					// title
	  }

Label {
	text: modelData.summary				// summary
	 }

Label {
	 text: modelData.updateTime				// updateTime
	  }

In the example in repo, if you click on each row of the list, the three properties should be changed
and the view should be updated(since I've coded to be so) but only "updateTime" is updated in View. If you comment out the third label and re-run the app, this time only "summary" is updated (the last one)

2- and the last item is updated only once
In the previous example, if you click on each row the last item is updated. But if you re-click, then the last item is no longer updated!

@SwiTool
Copy link

SwiTool commented Sep 26, 2019

In fact I can reproduce this too. The first signal sent makes the good display (the first one in fact), but when I change other datas (and call this.NotifySignal("xxxChanged") it does not update.

Here is the code snippet :

Main.qml

import QtQuick 2.7
import QtQuick.Controls 1.4
import B129 1.0
import "./Components"

ApplicationWindow {
    visible: true
    width: 800
    height: 600
    title: qsTr("B129")

    AccountsController {
        id: accCtrl

        Component.onCompleted: {
            accCtrl.initialize();
        }
    }

    AccountListItem {
        id: accountDelegate
    }

    Rectangle {
        width: 400
        height: parent.height

        ScrollView {
            anchors.fill: parent

            ListView {
                anchors.fill: parent
                model: Net.toListModel(accCtrl.accounts)
                delegate: accountDelegate
                focus: true
                width: parent.width
            }
        }
    }
}

AccountListItem.qml

import QtQuick 2.0
import QtQuick.Controls 2.5
import QtQuick.Layouts 1.0

Component {
    id: acc

    GridLayout {
        id: grid
        rows: 1
        columns: 1

        Text {
            id: position
            text: modelData.getState()
            font.pixelSize: 14
        }
    }
}

AccountsController.cs

using Qml.Net;
using System.Collections.Generic;
using System.Threading.Tasks;
using B129.Front.Interfaces;
using Model = B129.Front.Models;
using B129.Front.Extensions;

namespace B129.Front.Controllers
{
    public class AccountsController
    {
        private readonly IAccountService _accountService;
        private IList<Model.Account> _accounts = new List<Model.Account>();

        public AccountsController(IAccountService accountService)
        {
            _accountService = accountService;
        }

        public Task Initialize()
        {
            return RefreshAccounts();
        }

        [NotifySignal]
        public IList<Model.Account> Accounts
        {
            get { return _accounts; }
        }

        private async Task RefreshAccounts()
        {
            _accounts = (await _accountService.GetAccounts()).ToModel();
            foreach (var acc in _accounts) {
                System.Console.WriteLine($"Login: {acc.Login}, password: {acc.Password}");
            }
            Task.Run(() => {
                System.Console.WriteLine($"Waiting 5s...");
                System.Threading.Thread.Sleep(5000);
                System.Console.WriteLine($"Changed status");
                _accounts[0].AccountState = Logic.Enums.AccountState.CONNECT;
                _accounts[0].AccountState.ActivateNotifySignal(); // I don't know if it goes here but it did not do anything
                this.ActivateSignal("accountsChanged"); // this did nothing
                _accounts[0].AccountState.ActivateSignal("accountStateChanged") // this did nothing either
            });
            this.ActivateSignal("accountsChanged"); // it is the first, it worked
        }
    }
}

AccountModel.cs

using Qml.Net;
using B129.Logic.Enums;

namespace B129.Front.Models
{
    public class Account
    {
        public int Id {get; set;}

        [NotifySignal]
        public string Login {get; set;}
        public string Password {get; set;}
        
        [NotifySignal]
        public AccountState AccountState {get; set;}
        
        public string GetState()
        {
            return ((AccountState)AccountState).ToString();
        }
    } 
}

Result :

Login: mylogin, password: mypassword
Login: kiyuslebg, password: 123
Waiting 5s...
Changed status

image

Maybe i'm doing something wrong ?

@MosiQt
Copy link
Author

MosiQt commented Sep 26, 2019

Yeah, seems a bug. What do you think Paul? @pauldotknopf

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

Wow okay I got something. I think we're misusing the library.
Remember my line :

this.ActivateSignal("accountsChanged"); // it is the first, it worked

If I remove it, and let the one inside the Task.Run method, it still doesn't refresh. It stays empty.
I think the event loop refreshes the screen because of the Initialize method.

As I can see, his project here works and I can't find any difference with things we did...

@MosiQt
Copy link
Author

MosiQt commented Sep 27, 2019

Wow okay I got something. I think we're misusing the library.
Remember my line :

this.ActivateSignal("accountsChanged"); // it is the first, it worked

If I remove it, and let the one inside the Task.Run method, it still doesn't refresh. It stays empty.
I think the event loop refreshes the screen because of the Initialize method.

As I can see, his project here works and I can't find any difference with things we did...

Great. Anyway I'm not still get the solution. What's your solution to my example? (https://github.com/MosiQt/TestQmlDotNet.git)

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

For your example, try to call :

this.ActivateSignal("projectsChanged");

in your ViewModels/FeedViewModel.cs
instead of calling the project's NotifySignal.

I'll make more tests with my example to understand and will get back soon

// sorry I edited because I wrote this.NotifySignal instead of this.ActivateSignal

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

I think the problem is (in your example) that you have NotifySignal in a class that is already instancied and is already itself having the NotifySignal attribute.

I'm doing the same in my example : AccountsController has [NotifySignal]Accounts which contains itself properties with [NotifySignal]. I think this is a bad practice (even not a practice to say right since it doesn't work).

Correct me if i'm wrong

-- The only thing I don't get right now is why it does not refresh even by calling NotifySignal in a Task.Run method? I expect my data to be refreshed without any user interaction... Can you please tell ?

@vadi2
Copy link
Contributor

vadi2 commented Sep 27, 2019

I haven't looked into this issue, but I use .NET models, async computation, and lists in my app successfully: https://github.com/health-validator/Hammer so maybe look into there for differences too.

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

I haven't looked into this issue, but I use .NET models, async computation, and lists in my app successfully: https://github.com/health-validator/Hammer so maybe look into there for differences too.

@vadi2 You're using ActivateProperty, what is this for ? What is the difference with ActivateSignal ?

I tried to use it here, look what it did :
image
image

Note that it is called in the Task.Run method, in the Component.onCompleted method.

@vadi2
Copy link
Contributor

vadi2 commented Sep 27, 2019

You've got a class within a class? That won't work... already tried it. That's why my issues are now Lists: https://github.com/health-validator/Hammer/blob/master/Program.cs#L169

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

So when you update a single property in a class containing dozens of it, you must refresh the "master" class via the ActivateSignal("propertyNameOfClass") ?

@vadi2
Copy link
Contributor

vadi2 commented Sep 27, 2019

You can't nest classes within classes as you're saying - I've tried it, it doesn't work.

@vadi2
Copy link
Contributor

vadi2 commented Sep 27, 2019

health-validator/Hammer#87 is where we realised the problem and ditched that approach, because just as you, we ran into this issue of properties inside nested classes not updating (and we figure it's not meant to be).

@SwiTool
Copy link

SwiTool commented Sep 27, 2019

Alright, but why it doesn't update with the Task.Run method then ?

Task.Run(() => {
    System.Threading.Thread.Sleep(1000);
    System.Console.WriteLine($"Changed status");
    _accounts[0].AccountState = Logic.Enums.AccountState.CONNECT;
    this.ActivateSignal("accountsChanged");
});

image

@vadi2
Copy link
Contributor

vadi2 commented Sep 27, 2019

🤷‍♂️ I have not spent time investigating your usecase, sorry - but you're welcome to look at how Hammer does it, it works there.

@xsacha
Copy link

xsacha commented Nov 9, 2019

The way the signals work, it cannot travel multiple classes deep. This makes sense because it is listening to a parent change first. If the parent has not changed, it will not look at the children.

You can avoid this entire scenario and simplify your code by using a local binding to the child's class.
Then any signals to that class children will be correctly notified.

// Local binding to the child's class
property var v: myModel.view
ListView {
    // This will not work because 'view' doesn't get updated
    //model: Net.toListModel(myModel.view.dataModel);
    // This will automatically bind to changed in dataModel because it is only looking at dataModel
    model: Net.toListModel(v.dataModel);
}

@pauldotknopf
Copy link
Member

Hey guys, circling back to clean up issues.

First, I would avoid activating properties in background tasks. It should only be done in the GUI/Main thread.

Second, it's unclear if this issue is with Qml.Net, or the binding mechanism in Qml.

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

No branches or pull requests

5 participants