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

[Bug] SettingsLayoutItem Header Not Updating Dynamically and Fails to Display When Using Binding #432

Open
1 task done
SweetSmellFox opened this issue Mar 18, 2025 · 10 comments

Comments

@SweetSmellFox
Copy link

Check the following items

  • I have looked up relevant Issue

Description of the issue

The Header property of SettingsLayoutItem exhibits two critical issues:

​Static Display: The displayed header does not update when the Header value changes dynamically (e.g., through code).
code:

<suki:SukiWindow
    Title="AvaloniaTest"
    d:DesignHeight="450"
    d:DesignWidth="800"
    mc:Ignorable="d"
    x:Class="AvaloniaTest.MainWindow"
    x:DataType="avaloniaTest:ViewModel"
    xmlns="https://github.com/avaloniaui"
    xmlns:avaloniaTest="clr-namespace:AvaloniaTest"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:objectModel="clr-namespace:System.Collections.ObjectModel;assembly=System.ObjectModel"
    xmlns:suki="https://github.com/kikipoulet/SukiUI"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
    <Design.DataContext>
        <avaloniaTest:ViewModel />
    </Design.DataContext>

    <StackPanel>
        <suki:SukiStackPage Margin="20,0,20,20">
            <suki:SukiStackPage.Content>
                <suki:SettingsLayout>
                    <suki:SettingsLayout.Items>
                        <objectModel:ObservableCollection x:TypeArguments="suki:SettingsLayoutItem">
                            <suki:SettingsLayoutItem Header="TestA" x:Name="LayoutItem">
                                <suki:SettingsLayoutItem.Content>
                                    <StackPanel>
                                        <Button Click="Button_OnClick">Change</Button>
                                    </StackPanel>
                                </suki:SettingsLayoutItem.Content>
                            </suki:SettingsLayoutItem>
                        </objectModel:ObservableCollection>
                    </suki:SettingsLayout.Items>
                </suki:SettingsLayout>
            </suki:SukiStackPage.Content>
        </suki:SukiStackPage>
    </StackPanel>
</suki:SukiWindow>

using Avalonia.Interactivity;
using SukiUI.Controls;

namespace AvaloniaTest;

public partial class MainWindow : SukiWindow
{
    public MainWindow()
    {
        this.DataContext = new ViewModel();
        InitializeComponent();
    }


    private void Button_OnClick(object? sender, RoutedEventArgs e)
    {
        LayoutItem.Header = "TestB";
    }
}

video:

test.mp4

​Binding Failure: When binding the Header property (e.g., Header="{Binding MyHeader}"), the header text does not render at all, even if the bound property updates correctly.

View:

<suki:SukiWindow
    Title="AvaloniaTest"
    d:DesignHeight="450"
    d:DesignWidth="800"
    mc:Ignorable="d"
    x:Class="AvaloniaTest.MainWindow"
    x:DataType="avaloniaTest:ViewModel"
    xmlns="https://github.com/avaloniaui"
    xmlns:avaloniaTest="clr-namespace:AvaloniaTest"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:objectModel="clr-namespace:System.Collections.ObjectModel;assembly=System.ObjectModel"
    xmlns:suki="https://github.com/kikipoulet/SukiUI"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
    <Design.DataContext>
        <avaloniaTest:ViewModel />
    </Design.DataContext>

    <StackPanel>
        <suki:SukiStackPage Margin="20,0,20,20">
            <suki:SukiStackPage.Content>
                <suki:SettingsLayout>
                    <suki:SettingsLayout.Items>
                        <objectModel:ObservableCollection x:TypeArguments="suki:SettingsLayoutItem">
                            <suki:SettingsLayoutItem Header="{Binding HeaderA}" x:Name="LayoutItem">
                                <suki:SettingsLayoutItem.Content>
                                    <StackPanel>
                                        <Button Command="{Binding ChangeCommand}">Change</Button>
                                    </StackPanel>
                                </suki:SettingsLayoutItem.Content>
                            </suki:SettingsLayoutItem>
                        </objectModel:ObservableCollection>
                    </suki:SettingsLayout.Items>
                </suki:SettingsLayout>
            </suki:SukiStackPage.Content>
        </suki:SukiStackPage>
    </StackPanel>
</suki:SukiWindow>

ViewModel:

using Avalonia.Collections;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using System.Collections.ObjectModel;

namespace AvaloniaTest;

public partial class ViewModel : ObservableObject
{
    [ObservableProperty] private string _headerA = "Header A!";
    bool isA = true;
    [RelayCommand]
    private void Change()
    {
        HeaderA = isA ? "Header A!" : "Header B!";
        isA = !isA;
    }
}

Screenshot:
Image

Package Version

6.0.1

Environment

Windows 11 22631

Expected Behavior

The displayed header should update dynamically when the Header property changes.
Binding to the Header property should display the bound value and update it reactively.

Reproduction

Define a SettingsLayoutItem with a static Header value (e.g., Header="Initial Header").
Dynamically update the Header value (e.g., via a button click event).
csharp
settingsLayoutItem.Header = "Updated Header";
Observe that the UI does not reflect the new header value.
Replace the static Header with a binding:
xml

Ensure DynamicHeader is initialized and updated correctly in the ViewModel.
Observe that the header remains blank despite valid binding.

Additional Information

No response

@Insire
Copy link
Contributor

Insire commented Mar 18, 2025

I had a look through the code of the SettingsLayout again and i identified why this wont work. The author decided to use the SettingsLayoutItem as template to generate other controls in code behind. Copying the current value from the SettingsLayoutItem.Header into a TextBlock.Text. And since thats a simple string, changes to the SettingsLayoutItem.Header will never propagate.

I believe a rewrite for the entire SettingsLayout class is in order to make this behave like a normal bindable control. I can't even offer you a workaround, because the existing code does not take advantage of the ObservableCollection<SettingsLayoutItem> thats exposed on the SettingsLayout.

TLDR: we need to break things, to make this work well.

@kikipoulet
Copy link
Owner

I had a look through the code of the SettingsLayout again and i identified why this wont work. The author decided to use the SettingsLayoutItem as template to generate other controls in code behind. Copying the current value from the SettingsLayoutItem.Header into a TextBlock.Text. And since thats a simple string, changes to the SettingsLayoutItem.Header will never propagate.

I believe a rewrite for the entire SettingsLayout class is in order to make this behave like a normal bindable control. I can't even offer you a workaround, because the existing code does not take advantage of the ObservableCollection<SettingsLayoutItem> thats exposed on the SettingsLayout.

TLDR: we need to break things, to make this work well.

I plead guilty on this one 👀

I didn't even expect that someone would use the SettingsLayout except me, so to use it as bindable ..

I'm not against a rewrite, but I think it will be a little difficult (Handling scroll, .. )

@Insire
Copy link
Contributor

Insire commented Mar 19, 2025

I had a look through the code of the SettingsLayout again and i identified why this wont work. The author decided to use the SettingsLayoutItem as template to generate other controls in code behind. Copying the current value from the SettingsLayoutItem.Header into a TextBlock.Text. And since thats a simple string, changes to the SettingsLayoutItem.Header will never propagate.
I believe a rewrite for the entire SettingsLayout class is in order to make this behave like a normal bindable control. I can't even offer you a workaround, because the existing code does not take advantage of the ObservableCollection<SettingsLayoutItem> thats exposed on the SettingsLayout.
TLDR: we need to break things, to make this work well.

I plead guilty on this one 👀

I didn't even expect that someone would use the SettingsLayout except me, so to use it as bindable ..

I'm not against a rewrite, but I think it will be a little difficult (Handling scroll, .. )

Yea, its not an easy task. I dont see a way to make this MVVM compliant, but one could perhaps manually add a binding between SettingsLayoutItem.Header and the corresponding generated TextBlock in code behind. The question is, how much time we want to invest in this, or how well we want to support this. The alternative is to sunset this control and archive the code somewhere, for ppl to look at and copy paste things, if they desire.

@Insire
Copy link
Contributor

Insire commented Mar 22, 2025

@SweetSmellFox can you check my PR here and whether this solves your issue? I tested this locally and it seems to work with changing the text in the textblock via code behind.

@SweetSmellFox
Copy link
Author

@SweetSmellFox can you check my PR here and whether this solves your issue? I tested this locally and it seems to work with changing the text in the textblock via code behind.

I have a small question. If the author doesn't merge the PR, how can I obtain the generated DLL after merging(((

@Insire
Copy link
Contributor

Insire commented Mar 23, 2025

You could build the code yourself and use it in your project the old fashioned way, by referincing the dll you build yourself.

edit: the CI builds also provide an artifact, if you dont like building sukiui yourself

@SweetSmellFox
Copy link
Author

You could build the code yourself and use it in your project the old fashioned way, by referincing the dll you build yourself.

edit: the CI builds also provide an artifact, if you dont like building sukiui yourself

I tested the changes, and the binding now works as expected. Thank you!

@SweetSmellFox
Copy link
Author

You could build the code yourself and use it in your project the old fashioned way, by referincing the dll you build yourself.

edit: the CI builds also provide an artifact, if you dont like building sukiui yourself

I tested it and found that using this code, you can achieve dynamic binding without converting the Header to a TextBlock.(

// SettingsLayoutItem
            public static readonly StyledProperty<string?> HeaderProperty =
                AvaloniaProperty.Register<SettingsLayoutItem, string?>(
                    nameof(Header),
                    defaultValue: string.Empty);
            
            public string? Header
            {
                get => GetValue(HeaderProperty);
                set => SetValue(HeaderProperty, value);
            }

// SettingsLayout
            var header = new TextBlock
            {
                FontSize = 17
            };
            header.Bind(TextBlock.TextProperty, new Binding(nameof(SettingsLayoutItem.Header))
            {
                Source = settingsLayoutItem
            });
            var border = new Border
            {
                Child = new GroupBox
                {
                    Margin = new Thickness(10, 20),
                    Header = header,
                    Content = new Border()
                    {
                        Margin = new Thickness(35, 12),
                        Child = settingsLayoutItem.Content
                    }
                }
            };
            var textBlock = new TextBlock
            {
                FontSize = 17
            };
            textBlock.Bind(TextBlock.TextProperty, new Binding(nameof(SettingsLayoutItem.Header))
            {
                Source = settingsLayoutItem
            });
            var summaryButton = new RadioButton()
            {
                Content = textBlock,
                Classes =
                {
                    "MenuChip"
                }
            };

@Insire
Copy link
Contributor

Insire commented Mar 23, 2025

You could build the code yourself and use it in your project the old fashioned way, by referincing the dll you build yourself.
edit: the CI builds also provide an artifact, if you dont like building sukiui yourself

I tested it and found that using this code, you can achieve dynamic binding without converting the Header to a TextBlock.(

// SettingsLayoutItem
            public static readonly StyledProperty<string?> HeaderProperty =
                AvaloniaProperty.Register<SettingsLayoutItem, string?>(
                    nameof(Header),
                    defaultValue: string.Empty);
            
            public string? Header
            {
                get => GetValue(HeaderProperty);
                set => SetValue(HeaderProperty, value);
            }

// SettingsLayout
            var header = new TextBlock
            {
                FontSize = 17
            };
            header.Bind(TextBlock.TextProperty, new Binding(nameof(SettingsLayoutItem.Header))
            {
                Source = settingsLayoutItem
            });
            var border = new Border
            {
                Child = new GroupBox
                {
                    Margin = new Thickness(10, 20),
                    Header = header,
                    Content = new Border()
                    {
                        Margin = new Thickness(35, 12),
                        Child = settingsLayoutItem.Content
                    }
                }
            };
            var textBlock = new TextBlock
            {
                FontSize = 17
            };
            textBlock.Bind(TextBlock.TextProperty, new Binding(nameof(SettingsLayoutItem.Header))
            {
                Source = settingsLayoutItem
            });
            var summaryButton = new RadioButton()
            {
                Content = textBlock,
                Classes =
                {
                    "MenuChip"
                }
            };

I've updated the PR accordingly.

@kikipoulet
Copy link
Owner

@SweetSmellFox can you check my PR here and whether this solves your issue? I tested this locally and it seems to work with changing the text in the textblock via code behind.

I have a small question. If the author doesn't merge the PR, how can I obtain the generated DLL after merging(((

Don't worry, you know the goal of the library is to have better controls, not keeping flawed logic ^^

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

No branches or pull requests

3 participants