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

Forms and validation #5460

Closed
danroth27 opened this issue Jan 25, 2018 · 24 comments
Closed

Forms and validation #5460

danroth27 opened this issue Jan 25, 2018 · 24 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components cost: L Will take from 5 - 10 days to complete Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating.

Comments

@danroth27
Copy link
Member

No description provided.

@fileman
Copy link
Contributor

fileman commented Feb 22, 2018

share models with dataannotations between blazor and backend

@RichardC64
Copy link

RichardC64 commented Apr 9, 2018

do you plan a support like ?
<form @onsubmit(()=>SubmitForm()) ...>

@jonathanperis
Copy link

jonathanperis commented May 4, 2018

Aye!

Please do this the same way it works today in ASP.NET Core! through Data Annotations, with the same locking system until all the rules informed are validated successfully. Blazor will be magnificent with this functionality

@Andrzej-W
Copy link

Don't forget about localization and cultures:

  • different date/time formats
  • different decimal/thousandth separators
  • translated error/validation messages
  • translated field labels, etc.

@vertonghenb
Copy link
Contributor

How about using FluentValidation?
https://github.com/JeremySkinner/FluentValidation

@jonathanperis
Copy link

Please, try to ship this with 0.5.0! What do you think?

@SteveSandersonMS SteveSandersonMS self-assigned this Aug 15, 2018
@SteveSandersonMS
Copy link
Member

Intending to do some early design work in 0.6.0 (not actually delivering the feature).

@vertonghenb
Copy link
Contributor

Not everyone is fond of the current data annotation validation feature since I do believe in the idea of keeping the domain model rather simplistic. Therefore I really suggest to have a validation method that returns a dictionary with the error messages but the method itself can be overwritten.
Currently I use FluentValidation (mentioned above) and a html span to set the error message (returned from the validation method) before the form is submitted.

@DNF-SaS
Copy link

DNF-SaS commented Aug 16, 2018

Nice to have feature, but I'd see other topics (like full debugger support) with more priority.

@danroth27
Copy link
Member Author

@DNF-SaS Thanks for the feedback! Blazor is actually a composition of two efforts: 1. The Blazor UI framework, and 2. .NET support for WebAssembly (via Mono). Debugging is being handled on the Mono side and the work is tracked in the https://github.com/mono/mono repo.

@MihaMarkic
Copy link
Contributor

I'm also all for open validation, so not tied to DataAnnotation or anything in particular. This is really important.

@RemiBou
Copy link
Contributor

RemiBou commented Aug 23, 2018

I implemented client side form validation with DataAnnotation. It's just a POC but it's working, you can find the details here : https://remibou.github.io/Client-side-validation-with-Blazor-and-Data-Annotations/. Please send feedback, I'll try to create a nuget package from it later.

@danroth27
Copy link
Member Author

Moving this out of 0.6.0.

@SteveSandersonMS
Copy link
Member

Here is some rough mockups based on templated components: https://gist.github.com/SteveSandersonMS/6ac9458e3fc5b49c199777627fff3fc8

There are so many different ways we could structure designs for this.

For those wondering why we moved it out of 0.6.0, as commented earlier, our only goal in 0.6.0 was to consider possible designs, which is what we have done. Real implementation will be later.

@RyoukoKonpaku
Copy link
Contributor

RyoukoKonpaku commented Sep 22, 2018

@SteveSandersonMS Seems promising, one comment though If we put required attributes on controls from the looks of it, it won't do anything I assume? Either way I do like the customizability of the rules for each so you can fine grain stuff.

As for the model centric style of Validations I think it'd be nice if I could somehow also use the Attributes on the shared DTO to validate them also on server-side going by the "Don't trust the client-side too much", it would make validations easier for those who uses .Net full stack.

I also like your idea of having a default template for the Model centric one that one could override using a Templated component tag perhaps with it's context used to register which property belongs to. Just throwing some ideas at the moment.

@RemiBou
Copy link
Contributor

RemiBou commented Sep 22, 2018

@SteveSandersonMS my 2 cents :

  • Template centric approach is IMHO not a good solution. One of the big benefit of Blazor is two-way binding between your Model and your UI. I'm currently working a lot with angular and I still don't understand why they created the formbuilder, it costs a lot of mapping code (eg : bugs) to do the bindings by hand (model => form, form => model). MVVM is really nice from a developer perspective. One more time : two ways binding in display AND in form is the real benefit of this kind of framework.
  • A attribute approach (like DataAnnotation) would be nice so you'll be able to share validation logic on client and server side without one single line of code (eg : bugs). You won't need any adapter (like the ones from Data Annotations and jqueryval) and it would increase app security as developers won't forget to implement server-side validation.

@Liander
Copy link

Liander commented Sep 22, 2018

I also tried using both a template and through a model but concluded that I would actually prefer using an extension mechanism on existing elements instead. For example extending the input element with a custom attribute that the user can implement to extract model information of their choice <input For="@DataSource(Model, x => x.NewTodo)" />, see https://github.com/aspnet/Blazor/issues/404#issuecomment-413037767.

Maybe the goals are a bit different. I didn’t take it further because I didn’t see any interests. I will open a separate issue for it to further explain.

@Liander
Copy link

Liander commented Sep 22, 2018

@natalie-o-perret
Copy link

I back @RemiBou attribute approach in the sense that it would avoid developing twice the same sort of validations on both client and server.

@BenjaminCharlton
Copy link

BenjaminCharlton commented Nov 12, 2018

Will Blazor validation allow async validation that runs on the server and synchronous validation for checking validity on the server too? And prioritize the sync validations over the async ones? For example, first validate that the email address is a valid email address (sync on the client) then check that it hasn't already been registered on the web site (async on the server).

Will it have the ability to show different content for invalid, valid and pending state? E.g.

<Invalid>You must choose a user name</Invalid>
<Pending>Checking that name is available</Pending>
<Valid>Great! It's available</Valid>

I think these are two good things about Angular that should be available in Blazor.

@aspnet-hello aspnet-hello transferred this issue from dotnet/blazor Dec 17, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 17, 2018
@aspnet-hello aspnet-hello added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one area-blazor Includes: Blazor, Razor Components labels Dec 17, 2018
@TheCakeMonster
Copy link
Contributor

TheCakeMonster commented Dec 20, 2018

Would it not be wise to build an implementation based on one of the existing rich client UI patterns, such as IDataErrorInfo, or better INotifyDataErrorInfo? It feels like you'd be rather reinventing the wheel if you didn't look at reusing an existing pattern. The only question would be which pattern I suppose!
Blazor seems like a rich client for the web age, so looking at patterns used in .NET rich clients seems very appropriate. It would also offer reuse of rules between UIs, and easier migration from Windows rich client UIs to Blazor/Razor Components, which would be a huge advantage.

@TheCakeMonster
Copy link
Contributor

I would be uncomfortable with rules having to be defined in the template. In my experience, this leads to a lot of duplication, as you often need to test validity on both sides of an API boundary, or on multiple UIs.
People have become more comfortable with rules on the model, made more popular by DataAnnotations on MVC.
Rules in the template is akin to what we had in WebForms, and it was a pain. That almost always resulted in duplication of rules.
Rules should be as low down in your application as you can comfortably push them, to achieve consistency.
ViewModels can be used by those who feel that applying rules to the model itself is to be avoided. Personally, it's not something I worry about, but ViewModels help for those who do.

@arivera12
Copy link

Taking a look of some peoples point of view, we got to keep this simple, not reinvent the wheels and to keep it simple and DataAnnotation is the way to go. I totally agree @RemiBou on his second bullet point. This would ensure that data will have same validations for both client and server sides. Why we just don't implement something like this that already exists

ModelState.IsValid

and then get the errors messages like

var allErrors = ModelState.Values.SelectMany(v => v.Errors.Select(b => b.ErrorMessage))

and the display the errors messages in a custom component like

<ValidationErrors Errors="allErrors" />

Based on UX the best approach would be to apply css colors to the input and display the error message below it as @RemiBou implementations and we need to take also in consideration internationalization stuff for errors messages, input labels, message dialogs, etc.

@rynowak rynowak added the Needs: Design This issue requires design work before implementating. label Jan 3, 2019
@rockfordlhotka
Copy link

There are a couple philosophies on validation rules, and I think it is important to keep them in mind when considering validation features in any UI framework.

One philosophy says that validation rules aren't real business rules, and they can (and should) be enforced by the UI, not by an official business layer. And I get that, especially in the js-based web world where the business layer is almost always in .NET/Java on a server, and whatever is running in the browser is (at best) a shadow copy of the real rules.

Another philosophy, widely used in the smart client frameworks over the past 20+ years, is that the UI shouldn't embody the rules at all, but rather should provide interfaces or events or something such that the business layer can report to the UI what is good/bad and the UI can decide how to display that information to the user.

I'm a strong advocate of the second philosophy, because I'm a strong advocate of separation of concerns and having an official business layer separate from the presentation. To make such separation possible it is necessary to have some abstraction so all the rules (including validation) run in the business layer, but have immediate results displayed to the user.

XAML is amazingly good at this via a combination of interfaces (IDataErrorInfo) and data binding to anything - including types that provide clean abstractions between sophisticated business logic and the presentation layer.

So my plea is that if you go down the road of building validation do-dads into the UI framework (as I suspect is inevitable), that you also make sure to support the more powerful model of keeping all business logic in a business layer and provide an abstraction layer so the results of rules can be displayed to the user by the presentation layer immediately.

After all, what we're talking about here is running the business layer (or parts of it) in the browser directly via wasm - so the direct analogy is to WPF. Having something comparable to the rich data binding and IDataErrorInfo concepts in WPF, but logically translated to Blazor and HTML is critical.

@mkArtakMSFT mkArtakMSFT added cost: L Will take from 5 - 10 days to complete 1 - Ready labels Feb 6, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, 3.0.0-preview3 Feb 6, 2019
@SteveSandersonMS SteveSandersonMS added Done This issue has been fixed and removed 2 - Working labels Feb 20, 2019
@rynowak rynowak mentioned this issue Mar 4, 2019
56 tasks
@mkArtakMSFT mkArtakMSFT removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates labels May 9, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components cost: L Will take from 5 - 10 days to complete Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating.
Projects
None yet
Development

No branches or pull requests