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

Generate properties for templates using TemplatePartAttribute #11934

Open
maxkatz6 opened this issue Mar 23, 2022 · 9 comments
Open

Generate properties for templates using TemplatePartAttribute #11934

maxkatz6 opened this issue Mar 23, 2022 · 9 comments

Comments

@maxkatz6
Copy link
Member

There is a new attribute ported from WPF.
It will be useful to have optional code generation for "parts" of templates.
As an example, user code:

[TemplatePart("PART_TextPresenter", typeof(TextPresenter))]
public partial class TextBox
{
    protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
    {
        base.OnAttachedToVisualTree(e);
            
        if (IsFocused)
        {
            TextPresenter?.ShowCaret(); // TextPresenter is generated
        }
    }
}

Additional generated code, adds new property and sets value to it:

#nullable enable
public partial class TextBox
{
    private TextPresenter? TextPresenter { get; set; }

    static TextBox()
    {
         TemplateAppliedEvent.AddClassHandler<TextBox>((control, args) =>
         {
             control.TextPresenter = args.NameScope.Get<TextPresenter>("PART_TextPresenter");
         });
    }
}

Problems:

  1. Only one static ctor can be created for class, if we want to handle TemplateAppliedEvent to fill these properties.
  2. Only one override of OnTemplateApplied method is possible, if we want to use it to fill these properties.

Which means, we most likely will need to modify Avalonia to add yet another way to read namescope after template was applied.

@maxkatz6
Copy link
Member Author

@kekekeks @worldbeater if you have any thoughts.

@robloo
Copy link
Contributor

robloo commented Mar 23, 2022

I think code generation is a good option here and like the ideas. I'll have to read more into what WinUI is doing when I first noticed this (#7432 (comment)). They are also generating an enum so there are no magic strings anywhere.

See: https://github.com/microsoft/microsoft-ui-xaml/blob/2a6d2d69c809c141ca7916c9730fd5305487501e/dev/TeachingTip/TeachingTipTemplatePartHelpers.h

There is also a pattern that is used for UWP that removes strings -- it uses nameof() of the fields instead. This wouldn't work for these ideas because someone still has to define the name whether it's a field or a string so this is just FYI:

    [TemplatePart(Name = nameof(ColorPicker.AlphaChannelSlider),          Type = typeof(ColorPickerSlider))]
    [TemplatePart(Name = nameof(ColorPicker.AlphaChannelTextBox),         Type = typeof(TextBox))]
    public partial class ColorPicker : Microsoft.UI.Xaml.Controls.ColorPicker
    {
        private TextBox           AlphaChannelTextBox;
        private ColorPickerSlider AlphaChannelSlider;

        protected override void OnApplyTemplate()
        {
            this.AlphaChannelTextBox = this.GetTemplateChild<TextBox>(nameof(AlphaChannelTextBox));
            this.AlphaChannelSlider = this.GetTemplateChild<ColorPickerSlider>(nameof(AlphaChannelSlider));
        }
    }

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/main/Microsoft.Toolkit.Uwp.UI.Controls.Input/ColorPicker/ColorPicker.cs

@kekekeks
Copy link
Member

How exactly would custom OnApplyTemplate logic work with such generated code? Should we generate a partial method to be called from OnApplyTemplate?

@robloo
Copy link
Contributor

robloo commented Mar 23, 2022

How exactly would custom OnApplyTemplate logic work with such generated code? Should we generate a partial method to be called from OnApplyTemplate?

This is the best solution I can think of as well. Just do it like InitializeComponent() in a separate partial class file. It might be better actually because it is opt-in for each control. They have to explicitly call the new method from OnApplyTemplate().

add yet another way to read namescope after template was applied.

Just pass the arguments.

protected void InitializeTemplateParts(TemplateAppliedEventArgs e)
{

}

@maxkatz6
Copy link
Member Author

This method can be added to the avalonia base control class, and called just before OnApplyTemplate. As an option. Probably with some "hide from intellisense" attributes. A bit hacky, but transparent for user.

protected virtual void InitializeTemplateParts(TemplateAppliedEventArgs e);

@robloo
Copy link
Contributor

robloo commented Mar 23, 2022

This method can be added to the avalonia base control class, and called just before OnApplyTemplate.

Good idea! That would be totally transparent as you said. I think we have a working solution all ideas together.

@worldbeater
Copy link
Contributor

worldbeater commented Mar 23, 2022

Regarding the implementation details of the new generator, I guess it is worth introducing a new class similar to AvaloniaNameGenerator named e.g. TemplatePartGenerator, then adding a new option to GeneratorOptions that would control if the new generator is enabled, and then invoking the generator in the composition root.

The code that scans the assembly and finds all classes annotated with the TemplatePart attribute can be found here https://github.com/reactivemarbles/ObservableEvents/blob/main/src/ReactiveMarbles.ObservableEvents.SourceGenerator/SyntaxReceiver.cs#L17 Most likely we'll need to implement ISyntaxReceiver that receives all relevant classes with TemplatePart attributes.

@worldbeater
Copy link
Contributor

worldbeater commented Nov 11, 2022

To sum up, for the following annotated class:

[TemplatePart("PART_Text", typeof(TextPresenter))]
[TemplatePart("PART_AcceptButton", typeof(Button))]
public partial class TextBox
{
    protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
    {
        base.OnApplyTemplate(e);
        this.InitializeTemplateParts(e);

        // Text.ShowCaret();
        // AcceptButton.IsEnabled = false;
    }
}

We are going to generate the following partial class:

public partial class TextBox
{
    private global::Avalonia.Controls.Presenters.TextPresenter Text;
    private global::Avalonia.Controls.Button AcceptButton;

    private InitializeTemplateParts(global::Avalonia.Controls.Primitives.TemplateAppliedEventArgs e)
    {
        Text = e.NameScope.Get<global::Avalonia.Controls.Presenters.TextPresenter>("PART_Text");
        AcceptButton = e.NameScope.Get<global::Avalonia.Controls.Button>("PART_AcceptButton");
    }
}

@robloo
Copy link
Contributor

robloo commented Nov 11, 2022

There was a lot of further discussion in #9093 related to how to define the string constants and then enforce them statically in XAML. I think that discussion isn't fully closed yet and it may influence what is done here. Feel free to jump into that conversation as well. That said, the TemplatePart attribute isn't going to change so however the string name gets into the attribute is irrelevant for you.

Concerning the code generation:

  1. I would consider using .Find instead of .Get to fail gracefully if control authors don't do what they need to do. However, that would require nullable types so maybe isn't worth it. We definitely need good exception handling and build outputs here to say which template part isn't found though.
  2. Technically instead of TextPresenter the control should be named just Text. Not all template parts currently have good names...

Edit:

  1. The implementation you have seems ok to me. However, it also isn't using any of the partial class ideas. Using a partial class shared with an internal implementation in Control would allow InitializeTemplateParts to be automatically called. Otherwise we are all going to have to remember to call it inside an OnApplyTemplate override-- which I don't mind since it's clearly opt-in.

@maxkatz6 maxkatz6 transferred this issue from AvaloniaUI/Avalonia.NameGenerator Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants