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

DisplayAsync to return actual pre-rendered IHtmlContent #15247

Merged
merged 12 commits into from
Feb 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ public async Task<IHtmlContent> ExecuteAsync(DisplayContext context)
return HtmlString.Empty;
}

// Check if the shape is Position Wrapper
if(shape is PositionWrapper wrapper)
{
return PositionWrapper.UnWrap(wrapper);
}

// Check if the shape is pre-rendered.
if (shape is IHtmlContent htmlContent)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ public Task<IHtmlContent> ShapeExecuteAsync(IShape shape)
return Task.FromResult<IHtmlContent>(HtmlString.Empty);
}

// Check if the shape is wrapper, return underlying IHtmlContent
if( shape is PositionWrapper wrapper)
{
return Task.FromResult(PositionWrapper.UnWrap(wrapper));
}

// Check if the shape is pre-rendered.
if (shape is IHtmlContent htmlContent)
{
Expand Down
26 changes: 23 additions & 3 deletions src/OrchardCore/OrchardCore.DisplayManagement/PositionWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Text.Encodings.Web;
Expand All @@ -8,7 +9,7 @@

namespace OrchardCore.DisplayManagement
{
public class PositionWrapper : IHtmlContent, IPositioned, IShape
public sealed class PositionWrapper : IHtmlContent, IPositioned, IShape
{
private readonly IHtmlContent _value;
public string Position { get; set; }
Expand All @@ -29,13 +30,13 @@ public class PositionWrapper : IHtmlContent, IPositioned, IShape

public IReadOnlyList<IPositioned> Items => throw new System.NotImplementedException();

public PositionWrapper(IHtmlContent value, string position)
private PositionWrapper(IHtmlContent value, string position)
{
_value = value;
Position = position;
}

public PositionWrapper(string value, string position)
private PositionWrapper(string value, string position)
{
_value = new HtmlContentString(value);
Position = position;
Expand All @@ -50,5 +51,24 @@ public ValueTask<IShape> AddAsync(object item, string position)
{
throw new System.NotImplementedException();
}

public static PositionWrapper TryWrap(object value, string position)
{
if (value is PositionWrapper wrapper)
Copy link
Member

Choose a reason for hiding this comment

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

is IPositioned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used PositionWrapper because return value must be both IPositioned and IShape

{
// Update the new Position
wrapper.Position = position;
return wrapper;
}
else if (value is IHtmlContent content)
return new PositionWrapper(content, position);
else if (value is string stringContent)
return new PositionWrapper(stringContent, position);
else
throw new System.NotSupportedException($"Type of {nameof(value)} must be either {nameof(String)}, {nameof(IHtmlContent)} or {nameof(PositionWrapper)}.");
Copy link
Member

Choose a reason for hiding this comment

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

TryWrap should it return a new wrapper in this case instead of throwing?

Copy link
Member

Choose a reason for hiding this comment

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

Should it unwrap if the position is "" ? Is that the meaning of "" in ThemeLayout.cs?

Copy link
Contributor Author

@ns8482e ns8482e Feb 21, 2024

Choose a reason for hiding this comment

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

Should it unwrap if the position is "" ? Is that the meaning of "" in ThemeLayout.cs?

It's default, I believe when empty it will sort based on the order of items added

TryWrap should it return a new wrapper in this case instead of throwing?

Throwing NotSupportedException because value is not either string, IHtmlContent or PositionWrapper. Position wrapper doesn't support any other object - unless we return null instead of throwing exception - please suggest @sebastienros

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to return null instead of throwing exception

}

public static IHtmlContent UnWrap(PositionWrapper wrapper)
=> wrapper._value;
}
}
8 changes: 4 additions & 4 deletions src/OrchardCore/OrchardCore.DisplayManagement/Shapes/Shape.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ public virtual ValueTask<IShape> AddAsync(object item, string position)

_items ??= [];

if (item is IHtmlContent)
if (item is IHtmlContent htmlContent)
Copy link
Member

Choose a reason for hiding this comment

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

Can't this whole if/else/else be a single TryWrap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

{
_items.Add(new PositionWrapper((IHtmlContent)item, position));
_items.Add(PositionWrapper.TryWrap(htmlContent, position));
}
else if (item is string)
else if (item is string stringContent)
{
_items.Add(new PositionWrapper((string)item, position));
_items.Add(PositionWrapper.TryWrap(stringContent, position));
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public override async Task ExecuteAsync()
await ThemeLayout.Zones["Content"].AddAsync(body);

// Pre-render all shapes and replace the zone content with it.
ThemeLayout.Zones["Content"] = new PositionWrapper(await DisplayAsync(ThemeLayout.Zones["Content"]), "");
ThemeLayout.Zones["Content"] = PositionWrapper.TryWrap(await DisplayAsync(ThemeLayout.Zones["Content"]), "");

// Render each layout zone.
foreach (var zone in ThemeLayout.Properties.ToArray())
Expand All @@ -46,7 +46,7 @@ public override async Task ExecuteAsync()
continue;
}

ThemeLayout.Zones[zone.Key] = new PositionWrapper(await DisplayAsync(shape), "");
ThemeLayout.Zones[zone.Key] = PositionWrapper.TryWrap(await DisplayAsync(shape), "");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ public ValueTask<IShape> AddAsync(object item, string position)
_sorted = false;
_items ??= [];

if (item is IHtmlContent)
if (item is IHtmlContent htmlContent)
{
_items.Add(new PositionWrapper((IHtmlContent)item, position));
_items.Add(PositionWrapper.TryWrap(htmlContent, position));
}
else if (item is string)
else if (item is string stringContent)
{
_items.Add(new PositionWrapper((string)item, position));
_items.Add(PositionWrapper.TryWrap(stringContent, position));
}
else
{
Expand Down
Loading