Skip to content

Commit

Permalink
Fix TreeDataGridTextCell changing TextCell.Value
Browse files Browse the repository at this point in the history
`TreeDataGridTextCell` listens to property changes of `ITextCell.Value`
to update its own `Value` property. The setter of `TreeDataGridTextCell.Value`
updates `ITextCell.Text` to the string representation of the new value.

This is done because the cell can be edited, and
`TreeDataGridTextCell.Value` is bound to `TextBox.Text` two-way when the
user wants to edit the cell.

However, `TextCell<T>.Text` which the setter of `TreeDataGridTextCell.Value`
will always set, doesn't respect the read-only status or the editing
status of the cell:

1) `TextCell<T>.Value` updates.
2) `TreeDataGridTextCell` reacts and sets `TreeDataGridTextCell.Value`
    to the string representation of the new value.
3) The setter of `TreeDataGridTextCell.Value` will set `TextCell<T>.Text`
    to the string representation.
4) The setter of `TextCell<T>.Text` will try to convert the text
   representation of the new value as the new value...

If it weren't for the fact that `RaiseAndSetIfChanged` does what it's
supposed to do, which is only raise if changed, this would be a loop
that goes on forever.

Users will get an exception if `T` of `TextCell<T>` fails to convert:
`Convert.ChangeType(value, typeof(T))`. This obviously isn't an issue
when `T` is `string` but it does have issues for base types like numbers
(localization issues), and custom types will almost always result in an
exception.
  • Loading branch information
erri120 committed Sep 4, 2024
1 parent 38dce08 commit 43dc03a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ public TextCell(
public string? Text
{
get => _value?.ToString();
set{
set
{
if (IsReadOnly || !_isEditing) return;

if (string.IsNullOrEmpty(value))
{
Value = default(T?);
Expand Down
38 changes: 34 additions & 4 deletions tests/Avalonia.Controls.TreeDataGrid.Tests/Models/TextCellTests.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Globalization;
using System.Reactive.Subjects;
using System.Text;
using System.Threading.Tasks;
using Avalonia.Controls.Models.TreeDataGrid;
using Avalonia.Data;
using Avalonia.Headless.XUnit;
using Avalonia.Media;
using Xunit;

namespace Avalonia.Controls.TreeDataGridTests.Models
Expand Down Expand Up @@ -95,5 +92,38 @@ public void Modified_Value_Is_Not_Written_To_Binding_On_CancelEdit()
Assert.Equal("initial", target.Value);
Assert.Equal(new[] { "initial" }, result);
}

[AvaloniaFact(Timeout = 10000)]
public void Setting_Text_Does_Not_Change_ReadOnly_Value()
{
var binding = new BehaviorSubject<BindingValue<CustomValueObject>>(value: new CustomValueObject(100));
var target = new TextCell<CustomValueObject>(binding, isReadOnly: true);

Assert.Equal(100, target.Value.Value);

// simulating TreeDataGridTextCell.OnModelPropertyChanged
target.PropertyChanged += (sender, args) =>
{
if (args.PropertyName == nameof(ITextCell.Value))
target.Text = target.Value.ToString();
};

target.Value = new CustomValueObject(42);
Assert.Equal(42, target.Value.Value);
}

private readonly struct CustomValueObject
{
private readonly int _value;

public CustomValueObject(int value)
{
_value = value;
}

public int Value => _value;

public override string ToString() => _value.ToString(CultureInfo.InvariantCulture);
}
}
}

0 comments on commit 43dc03a

Please sign in to comment.