[dev-v5] Initial work for Option and Select implementation #3469
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
vnbaaij
commented
Mar 4, 2025
vnbaaij
commented
- Update WC v3 version
- Initial work for FluentOption
- Initial work for FluentSelect
- Add Slot to FluentAvatar
- Fix and update tests
- Initial work for FluentOption - Initial work for FluentSelect - Add Slot to FluentAvatar - Fix and update tests
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 98.9%
|
dvoituron
requested changes
Mar 4, 2025
| /// Gets or sets the content to display in the description slot. | ||
| /// </summary> | ||
| [Parameter] | ||
| public string? Description { get; set; } |
There was a problem hiding this comment.
What is the difference between ChildContent and Description ? Maybe we can add this difference in the doc?
There was a problem hiding this comment.
See https://web-components.fluentui.dev/?path=/docs/components-dropdown-option--docs#start-content for the difference. Description is shown as an extra line under the ChildContent
There was a problem hiding this comment.
Probably better to have a
RenderFragment property? And a name like SubChildContent or DescriptionTemplate.
There was a problem hiding this comment.
Then we would enable people to do things like this:
Think it is better to keep it a simple string. Think of it like the Message we have on the FluentField
There was a problem hiding this comment.
Make sense 😀. And the dev can use the ChildContent to format the content.
Can you add an explanation in this property doc: to explain that is a message below the ChildContent content.
| <fluent-text as="span" size="200" slot="message"></fluent-text> | ||
| </fluent-field> | ||
|
|
||
| <fluent-field id="xxx" label-position="after" size="medium"> |
There was a problem hiding this comment.
Why the FluentCheckbox tests changed?
There was a problem hiding this comment.
The Field/label did not take the Size into account yet.
There was a problem hiding this comment.
I don't understand this. If this test was correct in the dev-v5 branch, why is it wrong now without a change in FluentField?
There was a problem hiding this comment.
I did not make a change in FluentField. I added slot (==AdditionalAttribute) in FluentCheckBox.razor. So the rendered html is different.
There was a problem hiding this comment.
I actually made the change outside of this PR already but didn't update the verified files...doing that in dev-v5 now
| <FluentOption Value="One">One</FluentOption> | ||
| <FluentOption Value="Two">Two</FluentOption> | ||
| <FluentOption Value="Three">Three</FluentOption> | ||
| <FluentOption Value="@("One")">One</FluentOption> |
There was a problem hiding this comment.
Why did you update "One" to "@("One")"?
There was a problem hiding this comment.
FluentOption is now generic. When using string as the type, you need to specify the values like this. But made some more changes so will check if it is still necessary
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.