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

Components: Forms and validation #7614

Merged
merged 70 commits into from Feb 20, 2019

Conversation

@SteveSandersonMS
Copy link
Member

commented Feb 15, 2019

OK, this is a pretty massive PR, so @rynowak / @javiercn / @pranavkm - would any of you like me to give you a 30-minute tour of the changes and new features? Or if you prefer just to dive into the code that's completely fine with me too.

Features

  • EditContext - this tracks metadata about an edit process (e.g., what's been modified, current validation messages, etc.). It doesn't do any actual validation itself, but rather has an API designed to let external systems hook into it. External things can get notification about when fields are edited so they can choose to validate if they want.
  • EditForm - this creates an EditContext for you and cascades it down to contents. It also provides extra events (OnValidSubmit, OnInvalidSubmit, though you can still use OnSubmit directly if you want to trigger the validation yourself based on your own lifecycle design).
  • DataAnnotationsValidator - this attaches data annotations support to an EditContext
  • ValidationSummary - like the one in MVC
  • Built-in input components (InputText, InputCheckbox, InputSelect, etc.). These provide default behavior of validating on edit and changing their CSS class to reflect field state. Some of them have useful parsing logic (e.g., InputDate and InputNumber handle unparseable values gracefully by registering them as validation errors). The relevant ones also support nullability of the target field (e.g., int?).

For usage examples, see the new test cases in the E2E BasicTestApp:

  • SimpleValidationComponent
  • TypicalValidationComponent
  • NotifyPropertyChangedValidationComponent

Still to do for preview 3

  • Make the build pass. Need to check with @natemcmaster what's needed to deal with the platform manifest error. Should be OK now.
  • Rebase on master. This is tricky because in the last few days, the repo stopped taking a dependency on DataAnnotations, so I need to put that back. And first I need approval to join the darc group, etc. Done
  • E2E tests. The test cases themselves are implemented - I just haven't written the Selenium automation for them yet. Done
  • Maybe implement a ValidationMessage component, which would be trivial. Or developers can implement it themseles. By design, these sorts of things don't require priviledged access to any internal APIs. Implemented.
  • Remove the manual usages of *Expression from the test cases once this enhancement gets merged into Razor: aspnet/AspNetCore-Tooling#213 Not waiting for this.

Further enhancements planned for after preview 3

  • Async validation. I have a design in mind, but there's only so much can get done in time for Preview 3, and it would be good to get some customer feedback before trying to do everything.
  • Maybe some way to configure a default validation system, so you don't have to put <DataAnnotationsValidator> in every EditForm if that's what you're using.
  • Maybe some way to override the default CSS classes on the built-in Input* components
  • Ideally (though nontrivial), support for splatting attributes on Razor Components. Then we could make the built-in input controls automatically pass through all extra attributes, and developers who subclass them can still get that attribute passthrough behavior. Until we do something like this, some basic use cases like setting a maxlength on InputText just aren't supported (or at least, you have to subclass InputText and put that feature on your subclass, which is inconvenient).
@SteveSandersonMS

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

@natemcmaster Is it OK for me to put back the dependency on System.ComponentModel.Annotations that was removed from this repo in the last few days? We are still depending on it transitively via Microsoft.Extensions.Options, so it seems like there's no reason why it shouldn't be referenced from shared framework assemblies. If I understand correctly, I need to use darc to do this, somehow.

@natemcmaster

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@SteveSandersonMS See https://github.com/aspnet/AspNetCore/blob/master/docs/ReferenceResolution.md#example-adding-a-new-dependency.

I removed it because projects don't need this <Reference> if they target netcoreapp3.0. But it looks like this PR adds the reference in a netstandard2.0 project, so yes, a direct dependency on System.ComponentModel.Annotations is okay to put back.

@Eilon Eilon added the area-mvc label Feb 15, 2019

@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/forms-and-validation branch from e8397d2 to 3cc40e4 Feb 19, 2019

@SteveSandersonMS SteveSandersonMS requested a review from dougbu as a code owner Feb 19, 2019

@SteveSandersonMS

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2019

@natemcmaster Changed as requested. Please leave a 5☆ review if satisfactory :)

@natemcmaster
Copy link
Member

left a comment

⭐️ ⭐️ ⭐️ ⭐️ ⭐️

@SteveSandersonMS

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2019

/AzurePipelines run all

@azure-pipelines

This comment has been minimized.

Copy link

commented Feb 19, 2019

No pipelines are associated with this pull request.

@SteveSandersonMS

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2019

/AzurePipelines Go home, you're drunk.

@azure-pipelines

This comment has been minimized.

Copy link

commented Feb 19, 2019

Command 'Go' is not supported by Azure Pipelines.

Supported commands
     help:
          Get descriptions, examples and documentation about supported commands
          Example: help "command_name"
     run:
          Run all pipelines or a specific pipeline for this repository using a comment. Use
          this command by itself to trigger all related pipelines, or specify a pipeline
          to run.
          Example: "run" or "run pipeline_name"

See additional documentation.
@rynowak

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Command 'Go' is not supported by Azure Pipelines.

Supported commands
help:
Get descriptions, examples and documentation about supported commands
Example: help "command_name"
run:
Run all pipelines or a specific pipeline for this repository using a comment. Use
this command by itself to trigger all related pipelines, or specify a pipeline
to run.
Example: "run" or "run pipeline_name"

See additional documentation.

boom

@SteveSandersonMS SteveSandersonMS merged commit 7a2dfd3 into master Feb 20, 2019

17 of 18 checks passed

AspNetCore-helix-test Build #20190219.49 has failed
Details
AspNetCore-ci Build #20190219.53 has test failures
Details
AspNetCore-ci (Build: Linux ARM) Build: Linux ARM succeeded
Details
AspNetCore-ci (Build: Linux ARM64) Build: Linux ARM64 succeeded
Details
AspNetCore-ci (Build: Linux Musl x64) Build: Linux Musl x64 succeeded
Details
AspNetCore-ci (Build: Linux x64) Build: Linux x64 succeeded
Details
AspNetCore-ci (Build: Windows ARM) Build: Windows ARM succeeded
Details
AspNetCore-ci (Build: Windows x64/x86) Build: Windows x64/x86 succeeded
Details
AspNetCore-ci (Build: macOS) Build: macOS succeeded
Details
AspNetCore-ci (Code check) Code check succeeded
Details
AspNetCore-ci (Test: ANCM IIS) Test: ANCM IIS succeeded
Details
AspNetCore-ci (Test: ANCM IISBackCompat) Test: ANCM IISBackCompat succeeded
Details
AspNetCore-ci (Test: ANCM IISExpress) Test: ANCM IISExpress succeeded
Details
AspNetCore-ci (Test: ANCM IISForwardCompat) Test: ANCM IISForwardCompat succeeded
Details
AspNetCore-ci (Test: Ubuntu 16.04 x64) Test: Ubuntu 16.04 x64 succeeded
Details
AspNetCore-ci (Test: Windows Server 2016 x64) Test: Windows Server 2016 x64 succeeded
Details
AspNetCore-ci (Test: macOS 10.13) Test: macOS 10.13 succeeded
Details
license/cla All CLA requirements met.
Details

@SteveSandersonMS SteveSandersonMS deleted the stevesa/forms-and-validation branch Feb 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.