umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

Filled in Property value is not displayed when using the fallback/default value option #12805

Open eshanrnh opened 2 years ago

eshanrnh commented 2 years ago

Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9

10.1.0

Bug summary

When using the fallback/default value option on a property field with a value, the property value is not rendered on the frontend.

https://user-images.githubusercontent.com/82437098/183615531-6e87bd54-1a09-49e2-8f29-637c0c337810.mp4

Specifics

No response

Steps to reproduce

  1. Create a document type with a property.
  2. Create a content node and add value to the property.
  3. Render the property on the frontend - It works fine.
  4. Add a default value to the property and check the frontend - The value is not rendered.

Expected result / actual result

The property value should be displayed on the frontend irrespective of a default value.

nul800sebastiaan commented 2 years ago

Hi Esha and thanks! I the future, to save me some time, can you please add a code sample? I had to pause your video on full screen and type the same line on the other screen. A little timesaver would've been great there!

I can reproduce this issue, it's because you're trying to fall back from a string to a IHtmlString and that doesn't convert. If you change it to something like @Model.Value("footer", fallback: Fallback.ToDefaultValue, defaultValue: "Hello there - this is the fallback"), it works just fine. How did you come to the conclusion you needed to fallback to a new HtmlString?

This is where the conversion fails and we return the default value of a string, which would be an empty string: https://github.com/umbraco/Umbraco-CMS/blob/v10/dev/src/Umbraco.Core/Extensions/PublishedPropertyExtension.cs#L39

nul800sebastiaan commented 2 years ago

I'll close this for now as I think it works as intended but I'm curious to hear how you came to new HtmlString and then we'll see if we need to update something like the docs.

eshanrnh commented 2 years ago

Hey @nul800sebastiaan,

I'll make sure to add the code sample in the future 🙂

The new HtmlString is generated in the code when you render the property in the template using Insert -> Value -> Add default Value. Attached is the screencast of the same.

https://user-images.githubusercontent.com/82437098/183905578-3126e935-94a0-4608-9fd4-3e26ea7d7fae.mp4

nul800sebastiaan commented 2 years ago

Okay, that is a bug then! Reopened this one, will ask the team.

ronaldbarendse commented 2 years ago

Umbraco includes both a Value() and Value<T>() extension method and by specifying the default value, it will infer the generic type and always use the Value<T>() method.

Because footerText uses the Umbraco.Textbox editor and its Property Value Connector (PVC) returns a string, but the default value is of type HtmlString, it is actually calling Value<HtmlString>("footerText", fallback: Fallback.ToDefaultValue, defaultValue: new HtmlString("Hello there - this is the fallback")). Because there's a value for this property, it won't use the fallback to the default value and because it can't convert the string to HtmlString it returns nothing (or more specifically default(HtmlString), which is null).

Removing the new HtmlString from the inserted code will cause the same issue when selecting a property that actually returns an HtmlString type (like the RTE). Maybe the Add default value button should open a dialog that allows you to select different types, like string, HtmlString, integers, decimals, dates, etc. or even code, so you can add more complex default values? Selecting an incompatible type would still result in the same issue, but we could warn users about that in the dialog...

jonat123 commented 1 year ago

@nul800sebastiaan @ronaldbarendse What is the status of this issue?

I think that Ronald's suggestion about being able to choose different types sounds like a good way to go (If possible) since right now you might add a default value which then won't show and not really have no way of knowing why it is not working 🤷‍♂️

kjac commented 1 year ago

Hi everyone 👋

I think what we should be asking ourselves here is: How could anyone know what the correct type of a default value is?

You need to be in a typed view, so you can rely on ModelsBuilder generated types to deduct the default value type, or be somewhat of an expert to know the type generated by any given property value converter. I may be presumptuous here, but I would argue that people using the built-in template editor for building templates likely aren't experts, and the template editor surely isn't typed at this point.

My point is... being able to choose between different value types for default values likely won't help those affected by this issue, if they have no idea which type to choose?

claushingebjerg commented 1 year ago

Still a bug in 12...

kjac commented 1 year ago

Hi everyone.

My previous comment still stands. How is the editor going to know what type to enter? A simple string value won't work for a lot of editors, and we can't really create a typed "mini-IDE" for building complex default values (i.e. HtmlString).

We would love input on this. Maybe we're missing something obvious. The plan for now is to remove the "default value" feature entirely in the new backoffice.