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

Allow default values to be set on all core property types #7859

Closed
PeteDuncanson opened this issue Mar 30, 2020 · 7 comments
Closed

Allow default values to be set on all core property types #7859

PeteDuncanson opened this issue Mar 30, 2020 · 7 comments
Labels

Comments

@PeteDuncanson
Copy link
Contributor

Seems you can set a default value for all property types when you set them up. It would be lovely to be able to set them at the point of creating them.

Example property editors (not exhaustive):

  • Radio lists
  • Color picker
  • Drop downs

This was raised in a twitter conversation between several of the community here:

https://twitter.com/pgregorynz/status/1244553927523045378

Although it is possible to set the default via C# or Javascript code (although its not documented anywhere officially) its felt it really should be an additional bit of UI at the point of creation.

@nul800sebastiaan
Copy link
Member

Thanks!

Content Templates (aka blueprints) exist for this reason. to make it easier to set defaults and even have multiple defaults choices. Is there something lacking from that?

Then there's a bigger question of, where do we define defaults (datatype level, property level) and how to deal with variants.

Setting defaults through C# gives you very great flexibility in not "just" setting a default, but setting the default for a specific context you're in.

Note: Easter season is upon us soon and we'll need to have a chat at HQ about this, which is going to take a while, but I'll be back. Please leave additional comments reflecting on the questions above so we can take that into account as well.

@nul800sebastiaan nul800sebastiaan added the state/needs-investigation This requires input from HQ or community to proceed label Mar 31, 2020
@ronaldbarendse
Copy link
Contributor

@nul800sebastiaan Content templates aren't a real suitable solution, as you'd need to create one for every document type and instruct editors to use them (which is the most difficult and time consuming of the two). They also can't be used to set default values on nested content elements (or DTGE/other element type/block editors).

I also find it a bit strange that the checkbox data type does have a default value (whether it's checked or not). It would make a lot of sense to have a default value for dropdowns/radio lists, especially when they're mandatory. 🤔

Setting default values from code is a great feature, but I've seen a lot of broken/incorrect implementations (e.g. always setting the default if the value is empty, instead of only when the content is created). It also requires a lot of boilerplate code, making it unneccesarily hard to setup.


I've created the following helper class and we use this in all our projects (shipped as internal NuGet package) to allow setting default values for specific properties/document types and optionally only when specific conditions are met:

/// <summary>
/// Helper class for <see cref="EditorModelEventManager" />.
/// </summary>
public static class EditorModelEventManagerHelper
{
	/// <summary>
	/// Sets the default value for the specified <paramref name="documentTypeAlias" />.
	/// </summary>
	/// <param name="sender">The sender.</param>
	/// <param name="e">The <see cref="EditorModelEventArgs{ContentItemDisplay}" /> instance containing the event data.</param>
	/// <param name="propertyAlias">The property alias.</param>
	/// <param name="defaultValue">The default value.</param>
	/// <param name="documentTypeAlias">The document type alias.</param>
	public static void SetDefaultValue(HttpActionExecutedContext sender, EditorModelEventArgs<ContentItemDisplay> e, string propertyAlias, object defaultValue, string documentTypeAlias)
		=> SetDefaultValue(sender, e, propertyAlias, defaultValue, new[] { documentTypeAlias });

	/// <summary>
	/// Sets the default value for the specified <paramref name="documentTypeAliasses" />.
	/// </summary>
	/// <param name="sender">The sender.</param>
	/// <param name="e">The <see cref="EditorModelEventArgs{ContentItemDisplay}" /> instance containing the event data.</param>
	/// <param name="propertyAlias">The property alias.</param>
	/// <param name="defaultValue">The default value.</param>
	/// <param name="documentTypeAliasses">The document type aliasses.</param>
	public static void SetDefaultValue(HttpActionExecutedContext sender, EditorModelEventArgs<ContentItemDisplay> e, string propertyAlias, object defaultValue, string[] documentTypeAliasses)
		=> SetDefaultValue(sender, e, propertyAlias, defaultValue, documentTypeAliasses.InvariantContains(e.Model.ContentTypeAlias));

	/// <summary>
	/// Sets the default value.
	/// </summary>
	/// <param name="sender">The sender.</param>
	/// <param name="e">The <see cref="EditorModelEventArgs{ContentItemDisplay}" /> instance containing the event data.</param>
	/// <param name="propertyAlias">The property alias.</param>
	/// <param name="defaultValue">The default value.</param>
	/// <param name="enabled">If set to <c>true</c> sets the default value.</param>
	public static void SetDefaultValue(HttpActionExecutedContext sender, EditorModelEventArgs<ContentItemDisplay> e, string propertyAlias, object defaultValue, bool enabled = true)
	{
		if (enabled)
		{
			foreach (var property in e.Model.Variants.Where(v => v.State == ContentSavedState.NotCreated).SelectMany(v => v.Tabs).SelectMany(t => t.Properties).Where(p => p.Alias.InvariantEquals(propertyAlias)))
			{
				property.Value = defaultValue;
			}
		}
	}
}

This makes configuring the default values a lot easier:

public class ApplicationComposer : ComponentComposer<ApplicationComponent>, IUserComposer
{ }

public class ApplicationComponent : IComponent
{
	public void Initialize()
	{
		// Set default hide from navigation (checkbox) on children of list views
		EditorModelEventManager.SendingContentModel += (sender, e) => EditorModelEventManagerHelper.SetDefaultValue(sender, e, Constants.Conventions.Content.NaviHide, 1, e.Model.IsChildOfListView);

		// Set default SEO priority (slider) to 1 on home
		EditorModelEventManager.SendingContentModel += (sender, e) => EditorModelEventManagerHelper.SetDefaultValue(sender, e, "seoPriority", "1.0", Home.ModelTypeAlias);

		// Set nested content default values
		EditorModelEventManager.SendingContentModel += (sender, e) => EditorModelEventManagerHelper.SetDefaultValue(sender, e, "type", "Normal", new[] { CaseStudyImage.ModelTypeAlias, CaseStudyQuote.ModelTypeAlias, CaseStudySlider.ModelTypeAlias, CaseStudyTextBlock.ModelTypeAlias, CaseStudyTextBlockCentered.ModelTypeAlias });
		EditorModelEventManager.SendingContentModel += (sender, e) => EditorModelEventManagerHelper.SetDefaultValue(sender, e, "imageAlign", "Right", CaseStudyTextBlock.ModelTypeAlias);
	}

	public void Terminate()
	{ }
}

The helper class could also be extended to allow hiding the URLs/links (and preview button), removing properties or making them readonly.

If a lot of people could benefit from having this merged into the core, I'll create a PR for it 👍

@nul800sebastiaan
Copy link
Member

@ronaldbarendse - This does confirm that we need to do more work to consider what default values are and how they work. Please don't create a PR for this at the moment, we won't merge without having discussed this at HQ first and deciding if, what and how we want to do this.

@nul800sebastiaan
Copy link
Member

Apologies for the delay while we had some chats about this at HQ.

We love the ideas presented! Right now, we have our hands full but we'd be happy if a group of community members would like to work on this with our guidance.
As you know there's are a few community projects like the accessibility project (#5277) and the AngularJS decoupling project (#7718) and we like how these are being organized. Typically we see a list of PRs with progress or a Trello board with things to work on so that we have once central issue on the tracker that can spawn many small incremental PRs.

This feature is a little bit different in that we see no real easy small wins (correct me if I'm wrong), either the feature gets fully implemented or it does not. So it will require a bit more planning.

We would like to propose that a first step would be to describe the moving parts we'll need, for example the essentials:

  • Migrations (a checkbox has a default value already for some reason)
  • UI/UX - how do we configure a doctype to provide default values
  • Some C#
    • Updates to document type storage (?) - beware of breaking changes
    • The code from Ronald above looks nice as well, it's related but can probably be done as a separate isolated PR
  • Some Javascript

Then there is nice-to-haves, which could be things like:

  • How to deal with language variants and other types of variants?
  • A templating system, for example {{nodeName}} would be the name of the node, should be in line with nested content IMO so that we don't re-invent the wheel (see Placeholder attribute for textstring #8098)

In order for HQ to be able to help move this along, it would be great to first have an idea of what would be involved so we can say: yep that looks right.

Maybe we can think of alternatives to doing a full implementation? I can't think of any at the moment. We already mentioned the alternatives of blueprints and custom code.

In short, anything that can make the steps for implementing this feature smaller is going to be much quicker to evaluate, merge, and move on.

@nul800sebastiaan nul800sebastiaan added community/up-for-grabs type/feature and removed state/needs-investigation This requires input from HQ or community to proceed labels May 25, 2020
@umbrabot
Copy link

Hi @PeteDuncanson,

We're writing to let you know that we've added the Up For Grabs label to your issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly PR team bot :-)

@Shazwazza
Copy link
Contributor

My 2c - let's please not use the Data Type editor to store 'default values'. It is inconsistent that some editors might have the ability to assign default values from with the data type editor, this is purely because that data type configuration supports that but there is no guarantee or requirement for any data type to do that, it is entirely just based on if someone added that functionality to the property editor but i don't think that its where the responsibility should be.

@umbrabot
Copy link

umbrabot commented Jul 7, 2022

Hiya @PeteDuncanson,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant
This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@umbrabot umbrabot added the status/stale Marked as stale due to inactivity label Jul 7, 2022
@umbrabot umbrabot closed this as completed Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants