Skip to content

Support for 'multiple' attribute in '<select>' elements.#33950

Merged
MackinnonBuck merged 23 commits into
mainfrom
t-mbuck/select-multiple-support
Jul 6, 2021
Merged

Support for 'multiple' attribute in '<select>' elements.#33950
MackinnonBuck merged 23 commits into
mainfrom
t-mbuck/select-multiple-support

Conversation

@MackinnonBuck
Copy link
Copy Markdown
Member

@MackinnonBuck MackinnonBuck commented Jun 29, 2021

Support for multiple attribute in <select> elements.
The onchange event will now provide an array of the selected elements via ChangeEventArgs when the multiple attribute is specified, and array values can be bound to the value attribute.

PR Description

  • Added support for converting to/from array types in BindConverter.cs.
  • Added special handling of <select> elements with type select-multiple in BrowserRenderer.ts.
  • Updated the InputSelect component to infer the multiple attribute based on TValue.
  • Added E2E tests.

Addresses #5519

@MackinnonBuck MackinnonBuck requested a review from a team as a code owner June 29, 2021 22:57
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jun 29, 2021
@MackinnonBuck MackinnonBuck marked this pull request as draft June 29, 2021 23:02
Comment thread src/Components/Components/src/BindConverter.cs Outdated
Comment thread src/Components/Web/src/Forms/InputSelect.cs Outdated
@MackinnonBuck MackinnonBuck marked this pull request as ready for review June 30, 2021 00:50
@MackinnonBuck MackinnonBuck requested a review from Pilchie as a code owner June 30, 2021 01:13
Comment thread src/Components/Web.JS/src/Rendering/BrowserRenderer.ts Outdated
Comment thread src/Components/Web.JS/src/Rendering/BrowserRenderer.ts
Comment thread src/Components/Web/src/Forms/InputSelect.cs Outdated
Comment thread src/Components/Web/src/Forms/InputSelect.cs Outdated
Comment thread src/Components/Web/src/Forms/InputSelect.cs Outdated
Comment thread src/Components/test/testassets/BasicTestApp/SelectVariantsComponent.razor Outdated
Comment thread src/Components/test/testassets/BasicTestApp/SelectVariantsComponent.razor Outdated
Copy link
Copy Markdown
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

This is looking excellent, and very close to being done now I think :)

Comment thread src/Components/Components/src/BindConverter.cs Outdated
Comment thread src/Components/Shared/src/WebEventData.cs Outdated
Comment thread src/Components/Web.JS/src/Rendering/BrowserRenderer.ts Outdated
Comment thread src/Components/test/testassets/BasicTestApp/SelectVariantsComponent.razor Outdated
Comment thread src/Components/Components/src/ChangeEventArgs.cs Outdated
Comment thread src/Components/Web.JS/src/Rendering/BrowserRenderer.ts Outdated
Comment thread src/Components/Components/src/BindConverter.cs Outdated
@SteveSandersonMS
Copy link
Copy Markdown
Member

@MackinnonBuck Again, this looks super and very close to being done :) I found one more thing that probably definitely needs attention, and a "maybe" thing. Hopefully that's the last of it!

@DamianEdwards
Copy link
Copy Markdown
Member

This looks great! It would be amazing to get support for having a select element's items automatically populated with an Enum's values along with support for DisplayName for the text, when the select itself is bound to an Enum or IList<Enum> style property. It would make for a really compact and simple way to provide UX for enums, e.g.

<InputSelect @bind-Value="AThing">
</InputSelect>
<InputSelect @bind-Value="MultipleThings">
</InputSelect>
@code {
    Thing AThing;
    Thing[] MultipleThings;
    enum Thing
    {
        [DisplayName("This is the 1st thing")]
        FirstThing;
        [DisplayName("This is the 2nd thing")]
        SecondThing;
    }
}

If there's already an issue for this I couldn't find it! Happy to log one.

Copy link
Copy Markdown
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks so much for handling all the feedback. This looks like a really super and solid feature. Great to see you were able to eliminate the MultipleValues property too - this is a very clean API now!

@MackinnonBuck MackinnonBuck merged commit e65cec1 into main Jul 6, 2021
@MackinnonBuck MackinnonBuck deleted the t-mbuck/select-multiple-support branch July 6, 2021 16:19
@ghost ghost added this to the 6.0-preview7 milestone Jul 6, 2021
@campersau campersau mentioned this pull request Jul 29, 2021
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.

6 participants