fix(module:select): fix data source of type IEnumerable<object>

[anranruye]

🤔 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

💡 Background and solution

#1906 introduces a new approach to monitor DataSource change, but not handle the situation where ValueName is null. As a result, the following code which uses a DataSource with members of different types no longer works:

<Select DataSource="_data" @bind-Value="_value" Style="width:100px;"></Select>

@_value

@code{
    List<object> _data = new List<object> { 1, "2", 3d, true };
    object _value;
}

Another issue is, when we add a object of custom class to the IEnumerable<object> DataSource, THelper.ChangeType will fail. Sample code:

<Select DataSource="_data" @bind-Value="_value" Style="width:100px;"></Select>

@_value

@code{
    List<object> _data = new List<object> { 1, "2", 3d, true, new MyClass { Label = "Apple" } };
    object _value;

    class MyClass
    {
        public string Label { get; set; }

        public override string ToString()
        {
            return Label;
        }
    }
}

The simple demo code above may not have a real use-case, but obviouly we should make it work well.

📝 Changelog

Language	Changelog
🇺🇸 English	fix the data source which has members of different types
🇨🇳 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]

[anddrzejb]

Moving from setter cause this if block to loose at least a part of its usefulness and also is the cause why test Do_not_react_when_DataSource_not_changed() is failing. In the setter it was useful when setting DataSource from null to a non-null value. When executed in OnParametersSet() it is opposite - executes when DataSource is set to null when it was not null previously.

[anranruye]

No, I dont think so. I think they should be same before and after the change. Do_not_react_when_DataSource_not_changed() passed because you write the OnDataSourceChanged attribute after DataSource attribute(then first time in the DataSource setter, OnDataSourceChanged is null, OnDataSourceChanged?.Invoke() just does nothing), If we move OnDataSourceChanged to before DataSource, the test will fail, like this:

I add handlerCalled = false; before cut.SetParametersAndRender(parameters => parameters.Add(p => p.DataSource, _persons)); . Now the test will pass.

[anddrzejb]

Well...it seems you are right on both accounts. But you did clean up the if branch.
There is one thing though which I do not really like in Select and I was wondering what is your opinion on this.
I have an issue with OnDataSourceChange and with OnSelectedItemChanged/OnSelectedItemsChanged, namely that they are fired during initialization of the Select component. I think they should not fire when the component is constructed, which not is the case now...

[anranruye]

Sometimes I have the same feeling. But triggering the Change events during initialization may be useful when the users want to inject custom logic for the first render. One example is the Table OnChange(now it is used to get remote data). Of course this smells not good, however we should provide an alternative way to inject these logics if we want to change this behavior.

[anddrzejb]

Yeah, I get that, but that custom logic should be placed in the OnInitialized/OnInitializedAsync of the component using Select. For me, I do have logic in OnSelectedItemsChanged but I have to put extra _afterFirstRender boolean just to avoid running the logic in there during initialization. And this is in multiple places. I just have a feeling that more often you don't have to do any logic on initialization than you do.

[anranruye]

I agree.

[codecov]

Codecov Report

Merging #1932 (ba52556) into master (bb445ed) will increase coverage by 1.78%.
The diff coverage is 89.13%.

❗ Current head ba52556 differs from pull request most recent head 30e8d6b. Consider uploading reports for the commit 30e8d6b to get more accurate results

@@            Coverage Diff             @@
##           master    #1932      +/-   ##
==========================================
+ Coverage   23.78%   25.56%   +1.78%     
==========================================
  Files         484      507      +23     
  Lines       31448    24006    -7442     
  Branches        0      229     +229     
==========================================
- Hits         7480     6138    -1342     
+ Misses      23968    17832    -6136     
- Partials        0       36      +36     

Impacted Files	Coverage Δ
...ents/select/internal/DataSourceEqualityComparer.cs	57.14% <25.00%> (-5.36%)	⬇️
components/select/Select.razor.cs	54.20% <95.23%> (+4.26%)	⬆️
components/core/Helpers/MemberPath/PathNode.cs	46.15% <0.00%> (-8.85%)	⬇️
components/core/Helpers/THelper.cs	36.00% <0.00%> (-8.45%)	⬇️
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%)	⬇️
...ponents/core/Component/Overlay/type/TriggerType.cs	75.00% <0.00%> (-3.95%)	⬇️
components/input/TextArea.razor.cs	77.52% <0.00%> (-3.05%)	⬇️
... and 418 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 bb445ed...30e8d6b. Read the comment docs.