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

Don't assume HtmlString when inserting default values in templates #18144

Open
wants to merge 1 commit into
base: v15/dev
Choose a base branch
from

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Jan 28, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #12805

Description

When inserting default values in templates, the client always adds them as HtmlString - for example

@Model.Value("title", fallback: Fallback.ToDefaultValue, defaultValue: new HtmlString("Fallback for title"))

It ends up invoking Value<T>() method from FriendlyPublishedContentExtensions (source).

This results in a null value being rendered for any property editor whose value convert does not yield an HtmlString output - which, incidentally, none of the core property value converters seem to do.

In other words: The default value is rendered as long as the property is not filled out. As soon as the property has a value, the output becomes empty (null).

Given how the FriendlyPublishedContentExtensions work, the only way to make this "work" is by explicitly invoking the non-typed version of Value() (source) - in other words, rendering like this:

@Model.Value("title", fallback: Fallback.ToDefaultValue, defaultValue: (object)"Fallback for title")

I say "work" because this is actually functionally different if you were to insert markup as default value; as-is, that would be rendered as markup. With this change, the HTML would be rendered as raw text. However, given the circumstance, I feel this is a fair trade-off to a somewhat CompletelyBroken™ feature.

Future considerations

The "insert value" feature is only useful for simple properties that yield simple values (strings, numbers) and the RTE. Any other properties (like media, content pickers, blocks etc.) will render the ToString() value of whatever object their value converters yield.

For example, a media picker property rendered with @Model.Value(alias) yields a textual output along the lines of:

Umbraco.Cms.Core.Models.MediaWithCrops`1[Umbraco.Cms.Web.Common.PublishedModels.Image]

We should really consider if we want to have this feature in the template builder. It's a friendly feature for simple properties... right up until the point when it becomes unfriendly for other properties.

Testing this PR

  1. Create a document with a template. The document must have a text property.
  2. Use the template builder to insert the text property with a default value.
  3. Verify that the text property is rendered when filled out on the document.
  4. Verify that the default value is rendered when the text property is blank on the document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant