Skip to content

Commit dae6b7c

Browse files
niels9001Copilot
andcommitted
MarkdownTextBlock: address review feedback (PR #785)
- Revert WriteText buffer-growth change to pre-PR implementation; defer the allocation optimization to a follow-up (review comment). - Expose the rerender flag through Build(bool rerender = true) so callers can tune document reload behavior deliberately. - Clarify the theme-update coalescing intent in OnThemePropertyChanged / OnActualThemeChanged comments. - Make MyImage(HtmlNode, MarkdownTextBlock) control non-nullable and drop the related null suppression. - Shorten RichTextBlock <see cref> references in MyFlowDocument so the correct MUX/WUX type resolves via global usings. - Document that ImageMaxWidth/ImageMaxHeight default of 0 means 'no constraint'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c07f190 commit dae6b7c

6 files changed

Lines changed: 30 additions & 16 deletions

File tree

components/MarkdownTextBlock/src/MarkdownTextBlock.Properties.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,17 @@ private static void OnIsTextSelectionEnabledChanged(DependencyObject d, Dependen
157157

158158
// ── Theme DPs ───────────────────────────────────────────────────────
159159
// Defaults come from the XAML style. C# defaults are type-appropriate
160-
// fallbacks only. The shared property-changed callback batches multiple
161-
// simultaneous DP updates (e.g. theme switch) into a single re-render.
160+
// fallbacks only.
161+
//
162+
// The shared property-changed callback coalesces the burst of synchronous
163+
// DP updates produced by a single theme/style swap into one re-render. DP
164+
// callbacks fire synchronously on the UI thread, so the first update queues
165+
// a single re-render on the dispatcher and flips _themePropertyChangeQueued;
166+
// the remaining updates in the same burst are skipped by the flag. Because
167+
// the queued callback only runs after the current synchronous work drains,
168+
// ApplyText executes exactly once, after every property has been applied.
169+
// The flag is shared with OnActualThemeChanged so the theme event and the
170+
// DP updates it triggers collapse into the same single re-render.
162171

163172
private static void OnThemePropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
164173
{

components/MarkdownTextBlock/src/MarkdownTextBlock.xaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@
220220
<Setter Property="QuoteCornerRadius" Value="4" />
221221
<Setter Property="QuoteBarMargin" Value="0,0,4,0" />
222222

223-
<!-- Image properties -->
223+
<!-- Image properties (0 = no max constraint; image uses its natural / markdown-specified size) -->
224224
<Setter Property="ImageMaxWidth" Value="0" />
225225
<Setter Property="ImageMaxHeight" Value="0" />
226226
<Setter Property="ImageStretch" Value="Uniform" />

components/MarkdownTextBlock/src/MarkdownTextBlock.xaml.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ private void OnUnloaded(object sender, RoutedEventArgs e)
6969

7070
private void OnActualThemeChanged(FrameworkElement sender, object args)
7171
{
72+
// Coalesces with the theme DP callbacks (see OnThemePropertyChanged):
73+
// the shared flag collapses this event and the DP updates it triggers
74+
// into a single re-render queued on the dispatcher.
7275
if (!_themePropertyChangeQueued)
7376
{
7477
_themePropertyChangeQueued = true;
@@ -121,7 +124,7 @@ private void ApplyText(bool rerender)
121124
}
122125
}
123126

124-
private void Build()
127+
private void Build(bool rerender = true)
125128
{
126129
if (_renderer == null)
127130
{
@@ -155,6 +158,6 @@ private void Build()
155158
}
156159
_pipeline.Setup(_renderer);
157160

158-
ApplyText(true);
161+
ApplyText(rerender);
159162
}
160163
}

components/MarkdownTextBlock/src/Renderers/WinUIRenderer.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,14 @@ public void WriteText(string? text, int offset, int length)
118118
{
119119
if (length > _buffer.Length)
120120
{
121-
_buffer = new char[Math.Max(length, _buffer.Length * 2)];
121+
_buffer = text.ToCharArray();
122+
WriteText(new string(_buffer, offset, length));
123+
}
124+
else
125+
{
126+
text.CopyTo(offset, _buffer, 0, length);
127+
WriteText(new string(_buffer, 0, length));
122128
}
123-
text.CopyTo(offset, _buffer, 0, length);
124-
WriteText(new string(_buffer, 0, length));
125129
}
126130
}
127131

components/MarkdownTextBlock/src/TextElements/MyFlowDocument.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
namespace CommunityToolkit.WinUI.Controls.TextElements;
1616

1717
/// <summary>
18-
/// Represents a flow document that wraps a <see cref="Microsoft.UI.Xaml.Controls.RichTextBlock"/> for rendering markdown or HTML content.
18+
/// Represents a flow document that wraps a <see cref="RichTextBlock"/> for rendering markdown or HTML content.
1919
/// </summary>
2020
public class MyFlowDocument : IAddChild
2121
{
@@ -26,7 +26,7 @@ public class MyFlowDocument : IAddChild
2626
/// <summary>Gets or sets the text element (unused, required by <see cref="IAddChild"/>).</summary>
2727
public TextElement TextElement { get; set; } = new Run();
2828

29-
/// <summary>Gets or sets the underlying <see cref="Microsoft.UI.Xaml.Controls.RichTextBlock"/>.</summary>
29+
/// <summary>Gets or sets the underlying <see cref="RichTextBlock"/>.</summary>
3030
public RichTextBlock RichTextBlock
3131
{
3232
get => _richTextBlock;

components/MarkdownTextBlock/src/TextElements/MyImage.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,15 @@ public MyImage(LinkInline linkInline, Uri uri, MarkdownTextBlock control)
5151
}
5252
}
5353

54-
#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.
55-
public MyImage(HtmlNode htmlNode, MarkdownTextBlock? control)
56-
#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider declaring as nullable.
54+
public MyImage(HtmlNode htmlNode, MarkdownTextBlock control)
5755
{
5856
#pragma warning disable CS8601 // Possible null reference assignment.
5957
Uri.TryCreate(htmlNode.GetAttribute("src", "#"), UriKind.RelativeOrAbsolute, out _uri);
6058
#pragma warning restore CS8601 // Possible null reference assignment.
6159
_htmlNode = htmlNode;
62-
_imageProvider = control?.ImageProvider;
63-
_svgRenderer = control?.SVGRenderer ?? _defaultSvgRenderer;
64-
_control = control!;
60+
_imageProvider = control.ImageProvider;
61+
_svgRenderer = control.SVGRenderer ?? _defaultSvgRenderer;
62+
_control = control;
6563
Init();
6664
int.TryParse(
6765
htmlNode.GetAttribute("width", "0"),

0 commit comments

Comments
 (0)