Skip to content

[release/6.0-rc1] Binding support for 'bool' values with InputRadioGroup and InputSelect #35523

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 20, 2021
Merged

[release/6.0-rc1] Binding support for 'bool' values with InputRadioGroup and InputSelect #35523

merged 2 commits into from
Aug 20, 2021

Conversation

MackinnonBuck
Copy link
Member

Description

Added support for bool value bindings with the Blazor InputRadioGroup and InputSelect components.

Customer Impact

Having radio/select inputs with only two choices is a fairly common scenario (true/false, yes/no prompts). In these cases, bool is an appropriate type for the value bound to the selected choice.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

This change is almost purely additive, so it's unlikely that it will break other functionality.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Addresses #34816

@MackinnonBuck MackinnonBuck requested a review from a team as a code owner August 19, 2021 18:55
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Aug 19, 2021
@pranavkm
Copy link
Contributor

FYI @davidfowl for approval.

@davidfowl davidfowl added the Servicing-approved Shiproom has approved the issue label Aug 19, 2021
@davidfowl
Copy link
Member

Approved. I dunno if I should be using servicing approved 😄

@BrennanConroy
Copy link
Member

Looks like you didn't add a new API, but added an override which modified the unshipped file. Unfortunately this triggered the "don't modify APIs" check that we do for servicing branches.

We can probably modify

aspnetcore/eng/scripts/CodeCheck.ps1

Line 216 in 721c539

if ($targetBranch -like 'release*' -and $targetBranch -notlike '*preview*' -and $file -like '*PublicAPI.Unshipped.txt') {
to allow rc* branches

@BrennanConroy BrennanConroy requested a review from a team as a code owner August 19, 2021 21:56
BrennanConroy
BrennanConroy reviewed Aug 19, 2021
View reviewed changes
eng/scripts/CodeCheck.ps1
@@ -213,7 +213,7 @@ try {
}
}
# Check for changes in Unshipped in servicing branches
if ($targetBranch -like 'release*' -and $targetBranch -notlike '*preview*' -and $file -like '*PublicAPI.Unshipped.txt') {
if ($targetBranch -like 'release*' -and $targetBranch -notlike '*preview*' -and $targetBranch -notlike '*rc*' -and $file -like '*PublicAPI.Unshipped.txt') {
Copy link
Member

Choose a reason for hiding this comment

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

cc @wtgodbe

Copy link
Member

Choose a reason for hiding this comment

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

CC @dougbu as well

BrennanConroy
BrennanConroy previously requested changes Aug 19, 2021
View reviewed changes
Copy link
Member

@BrennanConroy BrennanConroy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm suspicious of this change, there are over 300 test failures in Components that look to have started around when this PR was being worked on.

You can see here on a Monday build of the PR https://dev.azure.com/dnceng/public/_build/results?buildId=1296055&view=results the tests were failing, and looking at any other test run of Components that don't have these changes there are nowhere near 300 test failures.

The reason you don't see any failures is because Steve accidentally switched the components pipeline to only run quarantined tests. You can see the 300 failures here #35484 where the tests are running again.

@MackinnonBuck
Copy link
Member Author

Hm, this is strange. Even if the tests surrounding this change broke, I'm not sure why it would cause 300 seemingly unrelated tests to fail.

I'll investigate this as soon as I can to find out if there's something here causing the issues.

@MackinnonBuck
Copy link
Member Author

MackinnonBuck commented Aug 20, 2021 •
edited
Loading

Running on my machine locally, all tests except 1 pass (NoHotReloadListenersAreOrdinarilyRegistered is the failure).

@BrennanConroy
Copy link
Member

Ok, after looking at more builds I think it's #35414 that caused the issues.

@BrennanConroy BrennanConroy dismissed their stale review August 20, 2021 00:59

Investigated

@wtgodbe wtgodbe enabled auto-merge (squash) August 20, 2021 20:25
@wtgodbe wtgodbe merged commit e8e2b5f into release/6.0-rc1 Aug 20, 2021
@wtgodbe wtgodbe deleted the t-mbuck/release/6.0-rc1 branch August 20, 2021 21:19
@ghost ghost added this to the 6.0-rc1 milestone Aug 20, 2021
dougbu added a commit that referenced this pull request Aug 24, 2021
@dougbu @dotnet-maestro
@github-actions @Tratcher @captainsafia @JamesNK @MackinnonBuck @BrennanConroy @rafikiassumani-msft
Merge from 'release/6.0-rc1' into 'release/6.0' (#35588)
057ed19 
* [release/6.0-rc1] Update dependencies from dotnet/runtime dotnet/efcore (#35513)

* Set HttpSys read error log levels to debug #35490 (#35542)

Co-authored-by: Chris R <[email protected]>

* [release/6.0-rc1] Treat reference type parameters in oblivious nullability context as optional (#35526)

* Treat parameters in oblivious nullability context as optional

* Only apply fix for reference types

* Update optionality check in API descriptor

* Update check in BindAsync and Mvc.ApiExplorer test

* HTTP/3: Use new QuicStream.ReadsCompleted property in transport (#35483)

Co-authored-by: James Newton-King <[email protected]>

* HTTP/3: Fix incorrectly pooling aborted streams (#35441)

* [release/6.0-rc1] Binding support for 'bool' values with InputRadioGroup and InputSelect (#35523)

* Binding support for 'bool' values with InputRadioGroup and InputSelect (#35318)

* Update CodeCheck.ps1

Co-authored-by: Brennan <[email protected]>

* [release/6.0-rc1] Update dependencies from dotnet/efcore dotnet/runtime (#35558)

* Update dependencies from https://github.com/dotnet/efcore build 20210820.19

Microsoft.EntityFrameworkCore.Tools , dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.Design
 From Version 6.0.0-rc.1.21420.8 -> To Version 6.0.0-rc.1.21420.19

* Update dependencies from https://github.com/dotnet/efcore build 20210820.22

Microsoft.EntityFrameworkCore.Tools , dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.Design
 From Version 6.0.0-rc.1.21420.8 -> To Version 6.0.0-rc.1.21420.22

* Update dependencies from https://github.com/dotnet/efcore build 20210820.30

Microsoft.EntityFrameworkCore.Tools , dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.Design
 From Version 6.0.0-rc.1.21420.8 -> To Version 6.0.0-rc.1.21420.30

* Update dependencies from https://github.com/dotnet/runtime build 20210820.15

Microsoft.NETCore.Platforms , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.Win32.SystemEvents , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.Extensions.Primitives , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.Extensions.Options , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Configuration , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging , Microsoft.Extensions.Http , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Hosting , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Ref , System.Windows.Extensions , System.Threading.Channels , System.Text.Json , System.Text.Encodings.Web , System.ServiceProcess.ServiceController , System.Drawing.Common , System.DirectoryServices.Protocols , System.Diagnostics.EventLog , System.Diagnostics.DiagnosticSource , System.IO.Pipelines , System.Security.Permissions , System.Security.Cryptography.Xml , System.Security.Cryptography.Pkcs , System.Runtime.CompilerServices.Unsafe , System.Resources.Extensions , System.Reflection.Metadata , System.Net.Http.WinHttpHandler , System.Net.Http.Json
 From Version 6.0.0-rc.1.21420.7 -> To Version 6.0.0-rc.1.21420.15

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Improve Minimal APIs support for request media types #35082 (#35230) (#35579)

* add support for request media types

* [release/6.0-rc1] Update dependencies from dotnet/efcore dotnet/runtime (#35573)

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Chris R <[email protected]>
Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: James Newton-King <[email protected]>
Co-authored-by: Mackinnon Buck <[email protected]>
Co-authored-by: Brennan <[email protected]>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rafiki Assumani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wtgodbe wtgodbe wtgodbe left review comments

@davidfowl davidfowl davidfowl approved these changes

@pranavkm pranavkm pranavkm approved these changes

@BrennanConroy BrennanConroy BrennanConroy left review comments

Assignees
No one assigned
Labels
area-blazor Includes: Blazor, Razor Components Servicing-approved Shiproom has approved the issue
Projects
None yet
Milestone
6.0-rc1
Development

Successfully merging this pull request may close these issues.
5 participants
@MackinnonBuck @pranavkm @davidfowl @BrennanConroy @wtgodbe