since #2772 change, I'm having issues with binding with trigger

[ChrisPulman]

It seems that since this change, I'm having issues with binding. For example:

• I have a textbox that I'm binding to some view model property
• When calling Bind(), I'm passing a trigger - in my case textBox.Events().LostKeyboardFocus()
• Now since this commit, when I'm opening my view the textbox remains empty (e.g. there's no initial update from vm to vw)
• When I give focus to the textbox and then move focus to the next control, the textbox is suddenly populated (since the trigger fires)

Has this scenario been tested? It's particularly weird since the trigger should indicate that something in the view has changed - it should not have any influence on the initial update from view model to view. Also see the review comment I added.

Originally posted by @ronaldvdv in #2772 (comment)

[ChrisPulman]

@ronaldvdv do you have a code snippet that you can share demonstrating your implementation?

From your description is sounds like your user control binding is not giving you an initial value, the operation from there on sounds like its working as expected. The Bind with Trigger should provide the bound control a value upon initial binding, then should only update each time the observable trigger has a new value.

[ronaldvdv]

Hi Chris,

First of all, thanks for moving my feedback to the right place on Github - apologies for not doing that myself :-)

I've investigated the issue some more in our application. I've also tested various other ReactiveUI versions between the old one we used (13.1.1) and the latest. It seems the issues are not there in 13.3.2 but they are introduced when upgrading to 14.1.1.

There's the issue mentioned above in some places, but not all textboxes. Note that there's two weird things here. Not just the fact that there's no initial value - but also the fact that the view trigger, causes the view to be updated. I would expect that the trigger would try to update the view model, since that's what it's purpose is, right? Instead, when I tab through the empty controls, they are suddenly populated without the view model setters being called. So, the trigger (lostfocus event) causes an update from view model to view, which seems the wrong way around.

There's also some other weird behaviour related to tab controls (which was not there in 13.3.2). Whenever I'm having two tabs, and each of these show a view for the same type of view model, then when switching tabs, the content of some controls on the tab are not updated. So, when I move from tab 1 to tab 2, some textboxes still keep their content related to the view model bound to tab 1.

I would love to get to the bottom of these issues (and help improve ReactiveUI as it's a great library!) but for now I have only limited budget and time to work on the project, so I cannot investigate much further yet. The application is rather large so it'll require a bit of time to create a small reproduction scenario. I hope to be able to do so soon; for now we'll need to revert to 13.3.2.

[ChrisPulman]

The following is the test that was used to validate the functionality where we check that the binding has the initial value, then only updates on the trigger.
Is there anything here that you can identify as being different to the way your code operates

[Fact]
        public void BindWithFuncToTriggerUpdateTest()
        {
            CompositeDisposable dis = new();
            PropertyBindViewModel? vm = new();
            var view = new PropertyBindView { ViewModel = vm };
            var update = new Subject<bool>();

            vm.JustADecimal = 123.45m;
            Assert.NotEqual(vm.JustADecimal.ToString(CultureInfo.InvariantCulture), view.SomeTextBox.Text);

            view.Bind(vm, x => x.JustADecimal, x => x.SomeTextBox.Text, update.AsObservable(), d => d.ToString(CultureInfo.InvariantCulture), t => decimal.TryParse(t, out var res) ? res : decimal.Zero).DisposeWith(dis);

            vm.JustADecimal = 1.0M;

            // value should have pre bind value
            Assert.Equal(view.SomeTextBox.Text, "123.45");

            // trigger UI update
            update.OnNext(true);
            Assert.Equal(view.SomeTextBox.Text, "1.0");

            vm.JustADecimal = 2.0M;
            Assert.Equal(view.SomeTextBox.Text, "1.0");

            update.OnNext(true);
            Assert.Equal(view.SomeTextBox.Text, "2.0");

            // test reverse bind no trigger required
            view.SomeTextBox.Text = "3.0";
            Assert.Equal(vm.JustADecimal, 3.0M);

            view.SomeTextBox.Text = "4.0";
            Assert.Equal(vm.JustADecimal, 4.0M);

            // test forward bind to ensure trigger is still honoured.
            vm.JustADecimal = 2.0M;
            Assert.Equal(view.SomeTextBox.Text, "4.0");

            update.OnNext(true);
            Assert.Equal(view.SomeTextBox.Text, "2.0");

            dis.Dispose();
            Assert.True(dis.IsDisposed);
        }

[ronaldvdv]

Hi Chris,

Looking at the test, it seems the trigger parameter is used to determine when the view model value should be copied to the view, right?

However, the documentation at https://www.reactiveui.net/docs/handbook/data-binding/ suggests the other way around - see section "Choosing when to update the source". We have a use case similar to what is stated there: we want to update the view model when the view lost focus.

Do you agree these are two opposite interpretations of the trigger parameter, or am I missing something? Not sure if this explains all behavior I'm seeing since the new release, but it's at least worth discussing :-)

Data Binding

[ChrisPulman]

Perfect, thanks for this, this stems from a test that was just passing but the test wasn't doing what it suggested, I matched the test to make that work, I will reinstate the original functionality in addition to this with a selector to select which direction the trigger will work in, I will default it to the original functionality.
Thanks very much for your feedback and patience.

[ronaldvdv]

Awesome - happy to test as soon as you have an update available :-) The other issue I mentioned may either be related or not; I'll investigate (and if needed, raise a separate issue) once we have the trigger direction fixed.

[ChrisPulman]

There is now an option to choose which direction you want, default is the original functionality pre V14

[glennawatson]

I'm aiming to get a release out next week sometime with these changes, it'll be a major version bump due to some other changes coming in.

[ronaldvdv]

Great, happy to test the release in our application!

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.