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

feat: default value attribute #308

Merged
merged 10 commits into from
Nov 16, 2023

Conversation

vicdotexe
Copy link
Contributor

Description

Implemented DefaultAttribute to enable initial values on model properties.

Fixes #297

Ensure that your pull request has followed all the steps below:

  • Code compilation
  • Created tests which fail without the change (if possible)
  • All tests passing (except a couple of net6 tests which weren't passing to begin with)
  • Extended the README / documentation, if necessary

@ascott18
Copy link
Collaborator

As I've thought about this more, I'm not sure this is the right solution to the problem at hand. I think the real problem here is the implicit required rule that I added for numbers whose range excludes zero after this discussion:

image

ce71b21#diff-3c5b2db008ce784d0530445b78d13bf07bd3e855765300a23205359a1d461ceaR61-R80

Populating default values in ModelConversionVisitor may cause unexpected issues that I don't think we want. There fundamental assumptions in coalesce-vue that convertToModel({}, meta) and likewise for mapToModel produce an object whose fields are all null.

If we do want to support DefaultValueAttribute, I think it should probably be a feature that only affects admin pages in create scenarios.

@ascott18
Copy link
Collaborator

Pushed some changes and will merge momentarily. The major change is to only populate default values in the viewmodel constructor, as this allows for prop dirty flags to behave as expected as well as preventing default values from interfering with other usages of models, like API response objects.

@ascott18 ascott18 merged commit 59de026 into IntelliTect:dev Nov 16, 2023
9 checks passed
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.

Attribute for default c-input values
2 participants