From 5cda49890086c9b940260cf1274f43b110ae5450 Mon Sep 17 00:00:00 2001
From: erri120 <erri120@fossmailer.de>
Date: Wed, 4 Sep 2024 14:39:45 +0200
Subject: [PATCH] Fix TreeDataGridTextCell changing TextCell.Value

`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.
---
 .../Models/TreeDataGrid/TextCell.cs           |  5 ++-
 .../Models/TextCellTests.cs                   | 38 +++++++++++++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/Avalonia.Controls.TreeDataGrid/Models/TreeDataGrid/TextCell.cs b/src/Avalonia.Controls.TreeDataGrid/Models/TreeDataGrid/TextCell.cs
index fe721f5b..2722b2c5 100644
--- a/src/Avalonia.Controls.TreeDataGrid/Models/TreeDataGrid/TextCell.cs
+++ b/src/Avalonia.Controls.TreeDataGrid/Models/TreeDataGrid/TextCell.cs
@@ -49,7 +49,10 @@ public TextCell(
         public string? Text
         {
             get => _value?.ToString();
-            set{
+            set
+            {
+                if (IsReadOnly) return;
+
                 if (string.IsNullOrEmpty(value))
                 {
                     Value = default(T?);
diff --git a/tests/Avalonia.Controls.TreeDataGrid.Tests/Models/TextCellTests.cs b/tests/Avalonia.Controls.TreeDataGrid.Tests/Models/TextCellTests.cs
index 44931703..6956d54f 100644
--- a/tests/Avalonia.Controls.TreeDataGrid.Tests/Models/TextCellTests.cs
+++ b/tests/Avalonia.Controls.TreeDataGrid.Tests/Models/TextCellTests.cs
@@ -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
@@ -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);
+        }
     }
 }