Skip to content

Use substitution values in image alt and title text #1163

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
May 20, 2025
Merged

Use substitution values in image alt and title text #1163

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a50013d
use substitution values in image alt and title text
colleenmcginnis Apr 21, 2025
b6d66fb
reorder imports
colleenmcginnis Apr 22, 2025
2c73209
fix when to resolve subs
colleenmcginnis Apr 22, 2025
6ff2c78
add image with subs to subs test
colleenmcginnis Apr 22, 2025
0647384
Merge branch 'main' into cmcg-issue-814
colleenmcginnis Apr 24, 2025
c0bb7a3
Merge branch 'main' into cmcg-issue-814
reakaleek May 19, 2025
5748ca0
Allow empty alt tag
reakaleek May 20, 2025
edd2e4c
Add title
reakaleek May 20, 2025
337e1bf
Refactor
reakaleek May 20, 2025
838c406
Update src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
reakaleek May 20, 2025
4774943
Update src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlinePa…
reakaleek May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ private static void WriteImage(HtmlRenderer renderer, ImageBlock block)
Label = block.Label,
Align = block.Align,
Alt = block.Alt,
Title = block.Title,
Height = block.Height,
Scale = block.Scale,
Target = block.Target,
Expand Down Expand Up @@ -144,6 +145,7 @@ private static void WriteFigure(HtmlRenderer renderer, ImageBlock block)
Label = block.Label,
Align = block.Align,
Alt = block.Alt,
Title = block.Title,
Height = block.Height,
Scale = block.Scale,
Target = block.Target,
Expand Down
11 changes: 9 additions & 2 deletions src/Elastic.Markdown/Myst/Directives/ImageBlock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information

using Elastic.Markdown.Diagnostics;
using Elastic.Markdown.Helpers;
using Elastic.Markdown.IO;

namespace Elastic.Markdown.Myst.Directives;
Expand All @@ -20,6 +21,11 @@ public class ImageBlock(DirectiveBlockParser parser, ParserContext context)
/// </summary>
public string? Alt { get; set; }

/// <summary>
/// Title text: a short description of the image
/// </summary>
public string? Title { get; set; }

/// <summary>
/// The desired height of the image. Used to reserve space or scale the image vertically. When the “scale” option
/// is also specified, they are combined. For example, a height of 200px and a scale of 50 is equivalent to
Expand Down Expand Up @@ -64,9 +70,10 @@ public class ImageBlock(DirectiveBlockParser parser, ParserContext context)
public override void FinalizeAndValidate(ParserContext context)
{
Label = Prop("label", "name");
Alt = Prop("alt");
Align = Prop("align");
Alt = (Prop("alt") ?? "{undefined}").ReplaceSubstitutions(context);
Copy link
Member

@reakaleek reakaleek Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain what this is doing?

Will this leave a {undefined} in the alt property?

Generally, there is valid use case for an empty alt="" property. See https://www.w3.org/WAI/tutorials/images/decorative/

Not sure if this still given with this condition.

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I stole borrowed from TabSetBlock.cs 🙃 but if an empty string is better, let's do that!

Copy link
Member

@reakaleek reakaleek Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting. I also don't understand what it's doing in TabSet :D

Copy link
Member

@reakaleek reakaleek Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mpdreamz does this have a specific purpose?

Copy link
Member

@Mpdreamz Mpdreamz Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh I can't think of any. Must have been a leftover from frenzy coding 😸

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having some trouble figuring out the right syntax. Just changing "{undefined}" to "" is the only thing I could get to work, but I don't think it makes sense to replace subs in an empty string.... Any hints? 😏

Copy link
Member

@reakaleek reakaleek May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like:

Prop("alt")?.ReplaceSubstitutions(context) ?? "";

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh the ? after Prop("alt") was the missing piece. Thanks!

Title = (Prop("title") ?? "{undefined}").ReplaceSubstitutions(context);

Align = Prop("align");
Height = Prop("height", "h");
Width = Prop("width", "w");

Expand Down
45 changes: 24 additions & 21 deletions src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,38 +63,41 @@ public override bool Match(InlineProcessor processor, ref StringSlice slice)

ValidateAndProcessLink(link, processor, context);

ParseStylingInstructions(link);
ParseStylingInstructions(link, context);

return match;
}


private static void ParseStylingInstructions(LinkInline link)
private static void ParseStylingInstructions(LinkInline link, ParserContext context)
{
if (!link.IsImage)
return;

if (string.IsNullOrWhiteSpace(link.Title) || link.Title.IndexOf('=') < 0)
return;
var attributes = link.GetAttributes();
var title = link.Title;

var matches = LinkRegexExtensions.MatchTitleStylingInstructions().Match(link.Title);
if (!matches.Success)
if (string.IsNullOrEmpty(title))
return;

var width = matches.Groups["width"].Value;
if (!width.EndsWith('%'))
width += "px";
var height = matches.Groups["height"].Value;
if (string.IsNullOrEmpty(height))
height = width;
else if (!height.EndsWith('%'))
height += "px";
var title = link.Title[..matches.Index];

link.Title = title;
var attributes = link.GetAttributes();
attributes.AddProperty("width", width);
attributes.AddProperty("height", height);
var matches = LinkRegexExtensions.MatchTitleStylingInstructions().Match(title);
if (matches.Success)
{
var width = matches.Groups["width"].Value;
if (!width.EndsWith('%'))
width += "px";
var height = matches.Groups["height"].Value;
if (string.IsNullOrEmpty(height))
height = width;
else if (!height.EndsWith('%'))
height += "px";

attributes.AddProperty("width", width);
attributes.AddProperty("height", height);

title = title[..matches.Index];
}
link.Title = (title ?? "{undefined}").ReplaceSubstitutions(context);
}

private static bool IsInCommentBlock(LinkInline link) =>
Expand Down Expand Up @@ -183,7 +186,7 @@ private static void ProcessCrossLink(LinkInline link, InlineProcessor processor,
s => processor.EmitError(link, s),
s => processor.EmitWarning(link, s),
uri, out var resolvedUri)
)
)
link.Url = resolvedUri.ToString();
}

Expand Down
1 change: 1 addition & 0 deletions src/Elastic.Markdown/Slices/Directives/_ViewModels.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class ImageViewModel
public required string? Label { get; init; }
public required string? Align { get; init; }
public required string? Alt { get; init; }
public required string? Title { get; init; }
public required string? Height { get; init; }
public required string? Scale { get; init; }
public required string? Target { get; init; }
Expand Down
40 changes: 40 additions & 0 deletions tests/Elastic.Markdown.Tests/Inline/SubstitutionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,43 @@ public void OnlySeesGlobalVariable() =>
public void HasNoErrors() => Collector.Diagnostics.Should().HaveCount(0);

}

public class ReplaceInImageAlt(ITestOutputHelper output) : InlineTest(output,
"""
---
sub:
hello-world: Hello World
---

# Testing ReplaceInImageAlt

![{{hello-world}}](_static/img/observability.png)
"""
)
{

[Fact]
public void OnlySeesGlobalVariable() =>
Html.Should().NotContain("alt=\"{{hello-world}}\"")
.And.Contain("alt=\"Hello World\"");
}

public class ReplaceInImageTitle(ITestOutputHelper output) : InlineTest(output,
"""
---
sub:
hello-world: Hello World
---

# Testing ReplaceInImageTitle

![Observability](_static/img/observability.png "{{hello-world}}")
"""
)
{

[Fact]
public void OnlySeesGlobalVariable() =>
Html.Should().NotContain("title=\"{{hello-world}}\"")
.And.Contain("title=\"Hello World\"");
}
Loading