-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Ensure it's possible to validate based on the fields in the UI #14426
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
Comments
Some thoughts about this from @mrpmorris: #10526 (comment) |
Hi all I've put together some changes for you to consider. They aren't in a format that I can submit a pull-request. What I've done is to copy the various form classes into a folder called This code should be backward compatible, but give the option for the user to specify I've done it this way so you can download the branch, and then run it to see it in action. Benefits are
The recursive attribute on the model doesn't sit right with my gut. It seems UI decisions are being imposed on the model being edited, but the model is imposing an approach on the UI. If an app wanted two ways of editing the same object (Expert=one form / New user=wizard UI) then a recursive attribute would prevent this by forcing one approach. PS: Ignore any changes with the text FormX or NewInputField. Those were corrections of earlier mistakes. https://github.com/mrpmorris/_BlazorValidationChanges_/pull/1/files |
Any thoughts on my suggested approach? If it has failings then I'd like to know so I can rethink. |
It's not something we're focusing on at the moment, as it's on the backlog and we have lots of urgent 3.1 work to get through. We'll consider possible designs in the future when we have capacity. Hope that's OK! |
Sure, thanks for your reply. I just wanted to make sure if it needed additional mental work that I was aware so I could do it! |
@SteveSandersonMS @danroth27 @pranavkm @rynowak I see this has been added to Blazor 3.1 -> https://github.com/aspnet/AspNetCore/blob/release/3.1/src/Components/Blazor/Validation/src/ObjectGraphDataAnnotationsValidator.cs I feel it is just introducing a different problem, where we end up with two solutions and neither of them solve all three validation scenarios. I want to ensure you've seen my comments on this: I did propose a solution that would deal with all three validation scenarios (the two you have now + wizard UI validating only visible components) here #14426 (comment) I'm not sure what you thought of my approach, but regardless of how you ultimately proceed I feel it would be better to leave this class out until a more complete solution has been identified. |
@mrpmorris Did you get any new feedback/comment for your solution or/and your suggestion to add the following code in #14972:
|
The suggestion was wrong, because lists and arrays live there, but it should skip String though. |
Potentially related: #31252 |
As this was a number of years ago, it seems I've deleted my proposed changes. The gist of it was that inputs register themselves with the EditContext by asking for its Field, and then you can have a validation strategy that only validates items that have fields in the EditContext. Please ask if that made no sense :) |
I think validation based on UI is very important mr @SteveSandersonMS sometimes it makes no sense to validate the whole model except what we are interested into showing... please make this possible! |
The out-of-box DataAnnotationsValidator follows Data Annotations conventions of doing model-based, not UI-based, validation.
Sometimes people may want UI-based validation instead (i.e., we validate just the fields that have a corresponding editor on the screen right now).
If we're missing APIs to enable customizations for this, add them.
The text was updated successfully, but these errors were encountered: