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

TargetNullValue & FallbackValue implemented #664

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@akgulebubekir
Copy link

akgulebubekir commented Jan 2, 2017

TargetNullValue & FallbackValue features added to Binding class

TargetNullValue: The value that is used in the target when the value of the source is null.
(This feature exists on Windows platforms. See more )

FallbackValue: Gets or sets the value to use when the binding is unable to return a value.
(This feature exists on Windows platforms. See more)

Usage:
<Label Text="{Binding User.Name, TargetNullValue='Unknown', FallbackValue='-'}"/>

  • Unknown: when User.Name is null
  • - : If BindingContext does not have property such User or Name

Bugs Fixed

API Changes

Added:

  • object : FallbackValue Property in BindingBase
  • object : TargetNullValue property in BindingBase

Behavioral Changes

-None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@dnfclas

This comment has been minimized.

Copy link

dnfclas commented Jan 2, 2017

Hi @akgulebubekir, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@StephaneDelcroix

This comment has been minimized.

Copy link
Member

StephaneDelcroix commented Jan 9, 2017

That https://forums.xamarin.com/categories/xamarin-forms-evolution is the right place to discuss new API proposition. This is usually only used to discuss the implementation.

  • TargetNullValue can be done with a Converter
  • What happens if TargetNullValue is not set ? does it fall back to FallBackValue ?
  • see previous question: there's no tests at all
  • the Clone() in TypedBinding will ignore those 2 properties
  • TargetNullValue is related to the Converter which is defined in Binding, not BindingBase. The same applies to FallBackValue which is related to the Path, and shouldn't be in BindingBase either. it is defined in BindingBase on other platforms, so it's fine
  • there are some unnecessary formatting change, but I don't really care about that.

For the reasons above, I'm going to close this PR. Don't get me wrong, I think it could be a nice addition, but this should be discussed first. So please open a proposal in the evolution forum. Thanks a lot.

@StephaneDelcroix

This comment has been minimized.

Copy link
Member

StephaneDelcroix commented Jan 9, 2017

I changed my mind. this PR is salvageable and doesn't require much of discussion, only agreement. I'm reopening for discussion

@dnfclas

This comment has been minimized.

Copy link

dnfclas commented Jan 9, 2017

Hi @akgulebubekir, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@StephaneDelcroix

This comment has been minimized.

Copy link
Member

StephaneDelcroix commented Jan 9, 2017

Note that having null by default as FallbackValue is a breaking change as right now we do not update the binding if the path can't be evaluated.

@akgulebubekir

This comment has been minimized.

Copy link
Author

akgulebubekir commented Jan 9, 2017

TargetNullValue can be done with a Converter
Consider this as a ** syntatic sugar ** that suppose to be in a language. Also Converter does not works if binded object does not implement INotifyPropertyChanged interface.

What happens if TargetNullValue is not set ? does it fall back to FallBackValue ?
Nope, FallBackValue does not get involve to binding process if binding path is correct and exists.

The Clone() in TypedBinding will ignore those 2 properties
Updated @StephaneDelcroix

TargetNullValue is related to the Converter which is defined in Binding, not BindingBase. The same applies to FallBackValue which is related to the Path, and shouldn't be in BindingBase either. it is defined in BindingBase on other platforms, so it's fine
I couldn't get your point

There are some unnecessary formatting change, but I don't really care about that.
Routine Visual Studio shortcut pressing :) CTRL - K + D

@StephaneDelcroix
Copy link
Member

StephaneDelcroix left a comment

This requires a truckload of unittests, and a proposal to mitigate the breaking change

@jamesl77

This comment has been minimized.

Copy link

jamesl77 commented Jan 26, 2017

TargetNullValue & FallbackValue are very useful, I hope they get implemented well and sooner(yesterday).
I need to write unnecessary code sometimes in both UI and view-model to overcome the lack of these.

@StephaneDelcroix

This comment has been minimized.

Copy link
Member

StephaneDelcroix commented Jul 24, 2017

Closing. It's breaking. We definitely consider adding this to the binding system, but not right now.

@ctrexler

This comment has been minimized.

Copy link

ctrexler commented Aug 31, 2017

I love everything your team has done. Just want to be another data point letting you know that this feature would be very appreciated, as I've encountered some trouble with this that I'm now forced to find some kind of workaround for. Thanks!

@opcodewriter

This comment has been minimized.

Copy link

opcodewriter commented Aug 31, 2017

Ditto. I have a long XAML background, and this feature is very useful, and like @ctrexler said, I also needed to find (not so nice) workarounds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.