Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

MultiBinding! (Issue 4565)#8684

Merged
StephaneDelcroix merged 2 commits intoxamarin:masterfrom
legistek:gh4565-multibinding
Apr 22, 2020
Merged

MultiBinding! (Issue 4565)#8684
StephaneDelcroix merged 2 commits intoxamarin:masterfrom
legistek:gh4565-multibinding

Conversation

@legistek
Copy link
Copy Markdown
Contributor

@legistek legistek commented Nov 26, 2019

Description of Change

Implements MultiBinding, per issue #4565.

fixes #4565

API Changes

Added:
public sealed class MultiBinding : BindingBase
public interface IMultiValueConverter

public static readonly object DoNothing { get; } in Binding
public static readonly object UnsetValue { get; } in BindableProperty
(the last two are used in IMultiValueConverter implementations)

Platforms Affected

All

Behavioral/Visual Changes

No changes to existing applications.

New applications can now use MultiBinding to bind a target property to a series of BindingBase expressions (including nested MultiBindings) that evaluate to a single value through an IMultiValueConverter instance provided by the application. For example:

<Button Text="Issue Driver's License">
     <Button.IsEnabled>
          <MultiBinding Converter="{StaticResource AllMustBeTrueMultiConverter}">
               <Binding Path="IsOver16"/>
               <Binding Path="HasPassedExam"/>
               <Binding Path="HasCriminalRecord" Converter="{StaticResource Inverter}"/>
          </MultiBinding>
     </Button.IsEnabled>
</Button>

All types of Bindings should be valid child bindings for the MultiBinding, including implicit source (BindingContext), explicit source Binding, relative source Binding, and even additional MultiBindings.

Except for allowing nested MultiBindings, all efforts have been made to match the WPF spec and behavior for the same feature.

Testing Procedure

Unit tests in Xamarin.Forms.Core

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

internal bool IsTarget { get; }

public static readonly BindableProperty ValueProperty = BindableProperty.Create(
"Value",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nameof(Value)

Comment thread Xamarin.Forms.Core.UnitTests/MultiBindingTests.cs Outdated
Comment thread Xamarin.Forms.Core.UnitTests/MultiBindingTests.cs
@samhouts samhouts added a/Xaml </> t/enhancement ➕ API-change Heads-up to reviewers that this PR may contain an API change labels Jan 29, 2020
@samhouts samhouts self-assigned this Jan 29, 2020
Copy link
Copy Markdown
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some remarks, otherwise 👍
thanks for the extensive test suite.

I know there's none in WPF, but what would be a good markup extension for this ? (we can bring that on a different PR)

Comment thread Xamarin.Forms.Core/MultiBinding.cs Outdated
Comment thread Xamarin.Forms.Core/MultiBinding.cs Outdated
public Collection<BindingBase> Bindings
{
get => _bindings ?? (_bindings = new Collection<BindingBase>());
set => _bindings = value;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should probably throw if applied, and also throw if the collection is modified once the binding is applied

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok on throw if applied. To detect collection modified wouldn't it have to be an INCC though? Is it worth it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there's none in WPF, but what would be a good markup extension for this ? (we can bring that on a different PR)

I was thinking about that myself, but I don't know of any precedent for a collection being represented through a markup extension, do you?

Comment thread Xamarin.Forms.Core/MultiBinding.cs
@AmrAlSayed0
Copy link
Copy Markdown

AmrAlSayed0 commented Feb 18, 2020

Is MultiBinding going to be in 4.5.0?
Edit: Sorry, didn't notice the "vNext (4.5.0)" 😅 project tag. It just that this PR seems to have been abandoned and the requested changes where not made. This is a relatively small (and not too costly) feature but has a really huge effect.

@legistek
Copy link
Copy Markdown
Contributor Author

@AmrAlSayed0 it has not been abandoned but I have been swamped with my actual job. I'm fine with @StephaneDelcroix's changes so if he or anyone wants to just make them and run with this that's fine by me. Otherwise I'll make them when I'm able.

@samhouts samhouts changed the base branch from master to 4.6.0 February 27, 2020 23:36
* Implement MultiBinding for OneWay and TwoWay modes

* Make it work with different BindingModes

* Add RelativeSource support.

* Additional tweaks for edge cases. Add unit tests.

* Implement RelativeSourceTargetOverride with a WeakReference

* Modify per PR feedback

Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>
@legistek legistek force-pushed the gh4565-multibinding branch from e2c3755 to d6a4985 Compare March 1, 2020 23:02
@legistek
Copy link
Copy Markdown
Contributor Author

legistek commented Mar 1, 2020

@StephaneDelcroix I think I'm finished here, just not sure if you actually want me to bother trying to detect binding collection changes through INCC.

This should now be properly rebased on 4.6.0 too btw.

@samhouts samhouts added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Mar 25, 2020
@samhouts samhouts changed the base branch from 4.6.0 to master April 16, 2020 17:09
@StephaneDelcroix StephaneDelcroix merged commit 573e6cd into xamarin:master Apr 22, 2020
@samhouts samhouts added the approved Has two approvals, no pending reviews, and no changes requested label Apr 22, 2020
@neobenedict
Copy link
Copy Markdown

Error Position 56:34. Type MultiBinding not found in xmlns http://xamarin.com/schemas/2014/forms

Version 4.6.0.726+446-sha.d0dc59903-azdo.3681414

Is this not usable yet?

@samhouts
Copy link
Copy Markdown
Contributor

samhouts commented May 2, 2020

This will be part of 4.7.0. You may try it in the latest nightly build. Thanks!

@ncarandini
Copy link
Copy Markdown

ncarandini commented Jun 11, 2020

I've made a Demo and it works.
For comparison and for those who don't like using converters and multi bindings I've added an example on how to create a property in the ViewModel that does the same thing.

Using multi binding requires using a converter that implement the logic of multi binding, while using a VM property that implement the same logic but require to notify that that property has changed when the other properties involved in the logic change their values.

Having the choice of using multi binding also in Xamarin Forms is a good thing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a/binding ⛓ a/Xaml </> API-change Heads-up to reviewers that this PR may contain an API change approved Has two approvals, no pending reviews, and no changes requested blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. Core F100 m/high impact ⬛ proposal-accepted t/enhancement ➕

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] MultiBinding

7 participants