Umbraco.Decimal Property Editor: Add Default value (closes #21787)#23015
Umbraco.Decimal Property Editor: Add Default value (closes #21787)#23015Jonty-Schmidt wants to merge 3 commits into
Conversation
|
Hi there @Jonty-Schmidt, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
|
I've tested this out @Jonty-Schmidt and looks to work as expected - thanks for the contribution.
As noted on the issue though, could you create a new issue to link this PR to, as I feel it really only addresses a comment you added to the issue rather than the issue itself. Unless of course you plan to extend it for integer too? I saw you left and retracted some comments @nielslyngsoe. Are you happy with this from a code perspective so that we can accept it please? I think we can also cherry-pick this for 17 as well. |
| propertyEditorUiAlias: 'Umb.PropertyEditorUi.Decimal', | ||
| config: [{ alias: 'step', value: '0.000001' }], | ||
| }, | ||
| { |
There was a problem hiding this comment.
Perhaps move this so it comes after the step setting? Seems like min and max (and arguably step) should go together as they are part of the same process of considering what valid values are.
| const minVerified = isNaN(min) ? 0 : min; | ||
| const defaultValue = this.#parseConfiguredNumber(config, 'defaultValue'); | ||
| if (defaultValue !== undefined) { | ||
| return value !== undefined ? value : defaultValue; |
There was a problem hiding this comment.
I note you can configure a default value that's outside of the min and max and therefore will be immediately invalid. We could look to clamp it, so if they default is lower than the min it'll use the min and if greater than the max it'll use the max. But I'm not sure it's necessary. When it comes to data type configuration validation we don't currently enforce that these settings are internally correct, and rely on the developer to get it right for their editors.
So no action here I feel, just noting.

Closes #21787, with regards to Umbraco.Decimal default values.
This adds an optional default-value to the Umbraco.Decimal property editor.
If configured, the datatype using this editor will default to that value rather than 0.