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

Checkboxes erroneously shown as failing validation #78

Closed
BillHally opened this issue Apr 15, 2019 · 5 comments · Fixed by #79
Closed

Checkboxes erroneously shown as failing validation #78

BillHally opened this issue Apr 15, 2019 · 5 comments · Fixed by #79

Comments

@BillHally
Copy link
Contributor

Description

Using the latest version (2.0.0-beta-9) of Elmish.WPF, if you bind to a DataGridCheckBoxColumn in a DataGrid, then changing its value results in an error being displayed by the validation template:

screenshot

Repro steps

  1. Create a DataGridCheckBoxColumn whose Binding is set to a 2-way Elmish.WPF binding.

  2. Click to change the value of the CheckBox

An example script can be found in the repository BillHally/Elmish.WPF.DataGridCheckBoxColumnIssue.

Expected behavior

CheckBox is displayed without incorrectly indicating that validation has failed.

Actual behavior

CheckBox is displayed incorrectly indicating that validation has failed.

Known workarounds

You could probably set a custom validation template that ignores errors (I've not tried).

Related information

This is caused by ViewModel.TrySetValue returning false. The reason it does so seems to be to avoid an updating issue.

However, I've performed some tests with the script I linked to above, and it seems that this issue was fixed by the addition of this changeset "adding UI thread synchronization option for .NET" to Elmish.

Consequently, I think that TrySetValue should be changed to return true when passed a binder whose name it recognises.

Making this change would resolve:

I'll send a pull request for the relevant changes.

@cmeeren
Copy link
Member

cmeeren commented Apr 15, 2019

However, I've performed some tests with the script I linked to above, and it seems that this issue was fixed by the addition of this changeset "adding UI thread synchronization option for .NET" to Elmish.

Ooh, nice! Yes, that makes sense.

@BillHally
Copy link
Contributor Author

Thanks for responding so quickly!

@cmeeren
Copy link
Member

cmeeren commented Apr 15, 2019

You say that this also resolves

TwoBinding updating issue #74 (providing that UpdateSourceTrigger is set to PropertyChanged)

But wasn't the point of that issue that it only applied to LostFocus?

@BillHally
Copy link
Contributor Author

I'm not sure why I thought it only fixed it when UpdateSourceTrigger is set to PropertyChanged. I've just tested it again, and (unless I'm misunderstanding what the expected behavior is) it behaves sensibly regardless of what it's set to.

@cmeeren
Copy link
Member

cmeeren commented Apr 16, 2019

Yes, you're right, I tested and it's fixed.

Three for one - thanks again!

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

Successfully merging a pull request may close this issue.

2 participants