egil / Htmxor

Supercharges Blazor static server side rendering (SSR) by seamlessly integrating the Htmx.org frontend library.
MIT License
109 stars 12 forks source link

Update renderer to correctly convert SwapStyle enum #53

Closed tanczosm closed 2 months ago

tanczosm commented 2 months ago

Originally posted by @egil in https://github.com/egil/Htmxor/issues/49#issuecomment-2110768943

The enum option has the advantage of not being "stringly-typed", but will require a small modification to the renderer such that if it is used directly in an attribute, it produces the correct markup.

E.g.: hx-swap=@SwapStyle.BeforeBegin renders as hx-swap="beforebegin".

That is a simple addition I can make to the renderer.

The SwapStyle enum can also be used as a starting point for the builder, which it already is, such that users can start with the enum and "pay the cost" of using the builder if they need the additional help.

I am in favor of changing the constants we that overlap with enums to internal, to make the API simpler for the end user.

egil commented 2 months ago

I have hit a snag on this. The RenderTreeBuilder calls ToString() on the attribute value when it builds the render tree, which effectively removes the original type so I can't detect it based on type information (https://source.dot.net/#Microsoft.AspNetCore.Components/Rendering/RenderTreeBuilder.cs,395).

It's still possible to special case this, e.g. by looking for attributes with the name hx-swap and then switching on the value adjusting the value as needed. But that is not a pretty solution.

egil commented 2 months ago

The alternative is simply to break with C# convention, and name the SwapStyle enum attributes like their string versions in HTML, e.g.:

/// <summary>
/// How to swap the response into the target element.
/// </summary>
public enum SwapStyle
{
    /// <summary>
    /// Default style is what is specified in <see cref="HtmxConfig.DefaultSwapStyle"/> for the application
    /// or htmx's default, which is <see cref="innerHTML"/>.
    /// </summary>
    Default,

    /// <summary>
    /// Replace the inner html of the target element.
    /// </summary>
    innerHTML,

    /// <summary>
    /// Replace the entire target element with the response.
    /// </summary>
    outerHTML,

    /// <summary>
    /// Insert the response before the target element.
    /// </summary>
    beforebegin,

    /// <summary>
    /// Insert the response before the first child of the target element.
    /// </summary>
    afterbegin,

    /// <summary>
    /// Insert the response after the last child of the target element.
    /// </summary>
    beforeend,

    /// <summary>
    /// Insert the response after the target element.
    /// </summary>
    afterend,

    /// <summary>
    /// Deletes the target element regardless of the response.
    /// </summary>
    delete,

    /// <summary>
    /// Does not append content from response (out of band items will still be processed).
    /// </summary>
    none,
}

I am leaning towards this being the best option.

tanczosm commented 2 months ago

This seems like the best option. Should we do the same for other htmx enum values to establish consistency?

egil commented 2 months ago

Should we do the same for other htmx enum values to establish consistency?

Yes, but let's also add a remarks section in the code docs for each where we explain why, i.e., that users are then able to use the attributes directly in markup.

tanczosm commented 2 months ago

Done. Ready for review.