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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(module:select): fix null value #1996

Merged
merged 2 commits into from
Oct 9, 2021

Conversation

anranruye
Copy link
Member

馃 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • Bundle size optimization
  • Performance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

馃敆 Related issue link

Fixes #1995
#1783

馃挕 Background and solution

馃摑 Changelog

Language Changelog
馃嚭馃嚫 English fix null value option
馃嚚馃嚦 Chinese

鈽戯笍 Self Check before Merge

鈿狅笍 Please check all items below before review. 鈿狅笍

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@github-actions
Copy link

github-actions bot commented Oct 3, 2021

@codecov
Copy link

codecov bot commented Oct 3, 2021

Codecov Report

Merging #1996 (223fa8b) into master (ca3e2e1) will increase coverage by 1.91%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1996      +/-   ##
==========================================
+ Coverage   25.38%   27.30%   +1.91%     
==========================================
  Files         488      511      +23     
  Lines       32098    24482    -7616     
  Branches        0      233     +233     
==========================================
- Hits         8148     6684    -1464     
+ Misses      23950    17762    -6188     
- Partials        0       36      +36     
Impacted Files Coverage 螖
...ents/select/internal/DataSourceEqualityComparer.cs 57.14% <33.33%> (+7.14%) 猬嗭笍
components/select/SelectOption.razor.cs 83.63% <100.00%> (+4.26%) 猬嗭笍
components/core/Helpers/MemberPath/PathNode.cs 46.15% <0.00%> (-8.85%) 猬囷笍
components/core/Reflection/TypeDefined.cs 77.77% <0.00%> (-7.94%) 猬囷笍
components/input/InputGroup.razor.cs 87.50% <0.00%> (-6.95%) 猬囷笍
components/tabs/TabPane.razor 86.66% <0.00%> (-6.20%) 猬囷笍
...ponents/date-picker/types/DatePickerPlaceholder.cs 22.72% <0.00%> (-4.20%) 猬囷笍
components/input/TextArea.razor.cs 77.52% <0.00%> (-3.05%) 猬囷笍
...ponents/core/Component/Overlay/type/TriggerType.cs 81.25% <0.00%> (-2.97%) 猬囷笍
components/timeline/Timeline.razor.cs 63.21% <0.00%> (-2.24%) 猬囷笍
... and 423 more

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update ca3e2e1...223fa8b. Read the comment docs.

Copy link
Member

@anddrzejb anddrzejb left a comment

Choose a reason for hiding this comment

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

Sorry to keep you waiting. I am a bit swamped.
I tested the changes using these reproductions of each of the issues:
#1783

<Select DataSource="@_list"
		@bind-Value="@_selectedValue1"		
		ValueName="@nameof(Person.Id)"
		LabelName="@nameof(Person.Name)"		
		Style="width:250px">
</Select>

@code {
    record Person(string? Id, string Name);   
    string? _selectedValue1 = null;

    List<Person> _list = new()
    {
        new Person(null, "John"),
        new Person("2", "Lucy"),
        new Person("3", "Jack"),
        new Person("4", "Emily")
    };         
}

#1995

<Select @bind-Value="UserAssignedId"
        TItemValue="string"
        TItem="string"
        DefaultActiveFirstOption="true"
        Style="width:250px">
    <SelectOptions>
        <SelectOption TItemValue="string" TItem="string" Value="null" Label="Not assigned" />
        <SelectOption TItemValue="string" TItem="string" Value="@("test")" Label="Test" />
    </SelectOptions>
</Select>

@code {
    string? UserAssignedId = null;
}
  1. The Select component will throw exception if the value is null聽#1783 reproduction does not show label for the null value after init:
    image

  2. Changing back to null value does not show label. The null value has to be selected once more for it to show in the Select input box:
    selectNullValueLabel

@anranruye
Copy link
Member Author

@anddrzejb I think this is the issue tracked by #1587. Do you think we should also fix it here?

@anddrzejb
Copy link
Member

Oh, you are correct, I forgot about that issue. It does seem to be directly connected with null value, that is why I thought it could be a part of this PR.

@anddrzejb
Copy link
Member

@anranruye are you planning to fix the visual issues I mentioned above in this PR? If not, I will approve it.

@anranruye
Copy link
Member Author

@anddrzejb Then let's close this one. I will fix that issue when I have time, in another pr.

@ElderJames ElderJames merged commit 6b1d4b8 into ant-design-blazor:master Oct 9, 2021
@anranruye anranruye deleted the FixSelectNullValue branch October 29, 2021 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SelectOption doesn't render properly if Value is null
3 participants