[Experiment] Markdig based MarkdownTextBlock

[nerocui]

This PR adds a new experiment: MarkdownTextBlock. It's a new take on the existing MarkdownTextBlock in the community toolkit. This implementation use the markdig library for parsing markdown syntax.

This initial implementation contains only early work. After this, the plan is to make it more generic, extensible and customizable.

screenshots

[CosmicPredator]

Looks good to me...!

[itsWindows11]

Also I guess might want to use ArrayPool and (ReadOnly)Span if possible here.

[michael-hawker]

Did a high-level pass, some initial comments. Some things we can do now, some things to think about for later.

[michael-hawker]

Ah, also looks like XAML Styler needs to be run, @nerocui you can run the script in the root with -Main and it should clean things up.

[niels9001]

I see that all styling happens in code behind.. would it make sense to move these to separate styles defined in XAML and we load those, when needed, in code behind? Not sure if there would be any performance implications though?

That would allow a developer to easily override a style (like we do here with the current MarkdownTextBlock for the Gallery itself).

[michael-hawker]

@nerocui I'll try and take this for a spin later this week. Any discussions you think we need to resolve before an initial merge?

When you have a chance, mind rebasing on the latest main?

You'll want to delete your AdditionalAssemblyInfo.cs file and remove the <Version> tag in your src csproj file.

[nerocui]

@michael-hawker I'll address the style in XAML issue later this week, but other than that I don't see anything else for the initial one.

[nerocui]

@michael-hawker the latest commit addressed all the comments so far. Please feel free to take another look.

[michael-hawker]

Why is this empty?

[michael-hawker]

This should be a property of the control, as otherwise you can't easily set it in XAML here.

Like someone's going to want to do:

<Page.Resources>
    <controls:MarkdownThemes x:Key="MyMarkdownStyle"
        Padding="8,4,0,4"
        H1FontSize="36" ... />
</Page.Resources>

<controls:MarkdownTextBlock MarkdownStyle="{StaticResource MyMarkdownStyle}"/>

Need some thought on the name of the class and property here I think. @niels9001 thoughts?

Also, not sure if straight properties will just work on the themes class. And if we want some defaults defined as resource keys in XAML as well... 🤔

[michael-hawker]

CI Error:

D:\a\Labs-Windows\Labs-Windows\components\MarkdownTextBlock\src\TextElements\MyList.cs(60,34): 
error CS8524: The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value. For example, the pattern '(CommunityToolkit.Labs.WinUI.MarkdownTextBlock.TextElements.BulletType)6' is not covered. [D:\a\Labs-Windows\Labs-Windows\components\MarkdownTextBlock\src\CommunityToolkit.WinUI.Controls.MarkdownTextBlock.csproj]

[michael-hawker]

Post-release of 8.0, we've seen a number of folks looking for the MarkdownTextBlock control.

@nerocui I know there's still some questions/discussions around structure/API, but if we could at least get a clean build, we could merge a preliminary version in with tasks in a tracking issue, and make it easier for other folks to start picking it up and providing feedback. Do you still have some time to help at least get it to that point? I don't know what else there may be beyond that warning as error I highlighted above. Thoughts?

[nerocui]

@michael-hawker I think that works. I haven't had time to work on this, but I can spend some time to refresh the branch and get to a clean build.

[nerocui]

Refreshed and it works locally. It's getting conflict in the submodule. I'm not sure how to resolve that.

[niels9001]

Refreshed and it works locally. It's getting conflict in the submodule. I'm not sure how to resolve that.

Could you try to update from main? Hopefully that would resolve the conflict

[nerocui]

@niels9001 I rebased the whole PR from main. Will try again.

[niels9001]

@Arlodotexe can you help out with the conflict?

@nerocui I think the tooling update isn't required for this PR to work, right?

[nerocui]

@niels9001 That's right. Tooling is not part of this change.

[michael-hawker]

@nerocui that's weird, you define the dependency in the dependencies.props so that's all that should be needed... Do you know if you reference the StyleDictionary anywhere?

@Arlodotexe any thoughts?

[nerocui]

@michael-hawker I added ColorCode.Core to dependencies.props, so hope that fixes things.

[michael-hawker]

@nerocui you already have that defined in your csproj as it's a net standard package, so it shouldn't have to be in the dependencies.props, that file is only for UWP/WinAppSDK/Uno differentiation. So we can put that back. There's something else going on, but not too sure what's going on. @Arlodotexe is going to take a look.

[JoeTomkinson]

@michael-hawker I added ColorCode.Core to dependencies.props, so hope that fixes things.

No luck, looks as though it's still failing on depenency-related errors to ColorCode.Core

[Arlodotexe]

I believe I've found the source of the issue. The reference to Microsoft.Toolkit.Uwp.UI.Controls.Markdown 7.1.2 on the UWP head contains a transitive reference to ColorCode.Core that's conflicting with the version being referenced in this component.

Removing the reference to the 7.x MarkdownTextBlock control and cleaning all references to it in the head, the build error is resolved. Only reproduces in Release mode, not Debug.

@michael-hawker We'll have to upgrade the heads in tooling to use the Labs version of MarkdownTextBlock over the 7.x version to resolve the build error.

[nerocui]

@michael-hawker We'll have to upgrade the heads in tooling to use the Labs version of MarkdownTextBlock over the 7.x version to resolve the build error.

Isn't that kind of chicken and egg problem? We'd need to merge this PR to have tooling to reference to.

[michael-hawker]

Ah, thanks for the insight @Arlodotexe. Another case of Toolkit building toolkit strikes again... any suggestions for a path for us to untangle that? Do we have to do an intermittent PR to the tooling submodule to disable the Markdown stuff or should we create a branch for the tooling here to get a clean CI pass all together and then just snap them together?

[nerocui]

What if in this PR I don't include ColorCode.Core? I'll just add it back in once this one merges and tooling is able to reference the new control.

[Arlodotexe]

@michael-hawker Considering that updates to the tooling will affect both the main repo and Labs, we should probably create a new branch in Tooling for this PR. We can set a ToolkitMarkdownTextBlockSourceProject in Build.Directory.props like we did here, and then set it up here so it can be consumed by the app head.

That will fix our CI issue here, but we won't be able to merge the tooling PR (or this one) into main until we have a NuGet package on the PR feed ready to be used in the CommunityToolkit/Windows repo for the other code path.

When we've got that sorted and can close this PR, we'll update tooling once more to use the package from the public NuGet feed instead of the PR feed.

[Arlodotexe]

What if in this PR I don't include ColorCode.Core? I'll just add it back in once this one merges and tooling is able to reference the new control.

Tested locally and can confirm that removing references to ColorCode in this component (both code and packages) also solves the build error.

If we go this route, we can postpone updating the tooling repo to use this new component until we've merged the PR, then we can re-add the ColorCode functionality in a later PR. @michael-hawker This seems more reasonable, what do you think?

[michael-hawker]

What if in this PR I don't include ColorCode.Core? I'll just add it back in once this one merges and tooling is able to reference the new control.

@nerocui that's not a bad idea. Would it be hard to isolate? @Arlodotexe sound good?

[nerocui]

What if in this PR I don't include ColorCode.Core? I'll just add it back in once this one merges and tooling is able to reference the new control.

@nerocui that's not a bad idea. Would it be hard to isolate? @Arlodotexe sound good?

I'll take a look tonight, but the usage of ColorCode is very limited. So it should be easy.

[nerocui]

Updated with colorcode removed. The code block won't render without it. I don't think we need to work around it for now since we are adding it right back after this right?

[niels9001]

Woohoo! Passed CI

[nerocui]

@michael-hawker shall we merge?

[Balkoth]

Will this be brought forward to the next release?

[michael-hawker]

@Balkoth right now this is just a labs component, it'll remain here until it gathers enough support/polish to be considered ready for release.

We should have a tracking issue to outline what phases/status this component is in, but it looks like we forgot to get that added before we approved and merged this. @Arlodotexe I think there may be a discussion, we should ensure we have both created or labeled properly and linked to in the markdown tags for the component. Though we should also evaluate how to best use the new tasklist/sub-issue stuff GitHub has now too to track work/issues needed for components in those parent issues.

[Balkoth]

This was already a component of the previous toolkit and not bringing it forward is an issue for me. Thats why i asked.

[michael-hawker]

@Balkoth this is the intended replacement. We've just been pretty focused on supporting Native AOT at the moment for our upcoming release. If you encounter issues trying to upgrade to the Labs Markdown component or a scenario not yet supported by this new one, please let us know. Feel free to file issues, suggest solutions, and open a PR. Contributions are welcome. Thanks!