Skip to content

Support nullable enum in InputSelect #19506

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

Merged
merged 3 commits into from
Mar 9, 2020

Conversation

GuillaumeNury
Copy link
Contributor

As BindConverter supports Nullable enums (here), call it if the underlying type has type of enum.

Addresses #13624

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Mar 2, 2020
@mkArtakMSFT
Copy link
Contributor

Thanks for your PR, @GuillaumeNury.
We will need to also have corresponding tests for this change. Please include those too so that we can move forward with this PR.

@mkArtakMSFT mkArtakMSFT requested review from pranavkm and removed request for SteveSandersonMS March 4, 2020 17:47
@dnfclas
Copy link

dnfclas commented Mar 5, 2020

CLA assistant check
All CLA requirements met.

@GuillaumeNury
Copy link
Contributor Author

I have added some tests. Feel free to tell me if anything needs to be added.

@@ -18,7 +18,7 @@ public abstract class InputBase<TValue> : ComponentBase, IDisposable
private readonly EventHandler<ValidationStateChangedEventArgs> _validationStateChangedHandler;
private bool _previousParsingAttemptFailed;
private ValidationMessageStore _parsingValidationMessages;
private Type _nullableUnderlyingType;
protected Type _nullableUnderlyingType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change to the public API surface and would need to go through an API review. This would mean changing this a property and documenting it, and I still don't think we would want to expose this on the API.

A much cheaper option would be to calculate this as a field in the InputSelect<TValue> type.

public class InputSelect<TValue> : InputBase<TValue>
{
    private readonly Type _nullableUnderlyingType = Nullable.GetUnderlyingType(typeof(TValue));
    ..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed ! I reverted that and used your solution.

@GuillaumeNury GuillaumeNury force-pushed the input-select-nullable-enum branch from a0c4741 to 000d86e Compare March 6, 2020 08:59
Copy link
Contributor

@pranavkm pranavkm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@pranavkm pranavkm merged commit 58c13c3 into dotnet:master Mar 9, 2020
@pranavkm pranavkm added this to the 5.0.0-preview2 milestone Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants