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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rewritten (module: select): Almost completely new - update 2 #800

Merged
merged 8 commits into from
Nov 27, 2020

Conversation

ElDiddi
Copy link
Contributor

@ElDiddi ElDiddi commented Nov 17, 2020

GitKraken didn't want to open my repository anymore, after rebuilding the repository Github doesn't recognize the old PR or didn't want to append the PR to the existing one. Therefore a new PR.

!!! This PR is a Breaking Change !!!
Users who work with the select-module have to change their code after the merge.

Whats new?

  • Datasource IEnumerable - @bind-Value and @bind-Values
  • Typeparams TItemValue, TItem
  • Two-Way binding for the modes default, multiple and tag.
  • OnDataSourceChange event when the DataSource changes
  • Decoupling the SelectOptions from the data model, maybe this would help for virtualization later. (virtualization is not implemented atm)
  • Keyboard support (Arrow keys up/down, Home, End & Enter for selection)
  • Sorting for groups and labels (None, Asc, Desc)
  • Reflection for Value, Label, Disabled, GroupName
  • EventCallback OnSelectedItemChanged and OnSelectedItemsChanged
  • Optional search for all three modes
  • CreateCustomTag Action, here the developer can add his own logic
  • LabelTemplate & ItemTemplate simplified
  • IgnoreItemChanges to improve performance through Reflection
  • The demos and the API description are completely revised.

-- Update 2 --

  • Simplification of the upgrade path ***
  • Re-Added the property LabelInValue but it generates a functional Json string that you can parse with Newtonsoft.Json or System.Text.Json. It is only available when the SelectOption created without a DataStore. If you use the DataStore binding, you get the TItem.
  • modified the CalendarHeader
  • Prettified some demos
  • Added the namespace AntDesign.Select.Internal for all classes which the user does not need.

*** Simplification
If you do not want to create the data binding using a DataSource, you can now use the ChildComponent <SelectOptions></SelectOptions> and create the SelectOption directly. This should avoid unnecessary source code which could be created by a DataSource binding.

<Select @bind-Value="@_selectedValue"
        TItemValue="int"
        TItem="string">
	<SelectOptions>
                <SelectOption TItemValue="int" TItem="string" Value=1 Label="Jack" />
                <SelectOption TItemValue="int" TItem="string" Value=2 Label="Lucy" />
                <SelectOption TItemValue="int" TItem="string" Value=3 Label="Yaoming" Disabled />
                <SelectOption TItemValue="int" TItem="string" Value=4 Label="Frieda" />
	</SelectOptions>		
</Select>

** Note ***
Don't use a inline initialization of the Datastore. Like this:

<select DataSource="@(new List<MyClass> { new MyClass { Id=1, Name="Lucy" } })"

There is certainly still plenty of room for improvement.
The documentation for the demos needs to be translated into Chinese.

-- Update 4 --

  • Added the old demos again to make the demos comparable to React/Angular.
  • Changed some PropertyNames
  • Fixed a bug which triggered the OnSelectedItemsChanged event twice.

🤔 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

PR affected the following issues:
DefaultValue of select works incorrect #235
Keyboard navigation #261
[select]Related select elements displaying issue #473
[select] Default value issue #508
[SELECT] form binding issue - can't reset #515
关于 select 的问题 #695 <-- should be fixed, worked for me
[Select] Please add TValue for value type conversion #724
bug(module: select): Custom tags cannot be created if a part of the tag lable is already in use #727
bug(module: select): Custom tags are not unchecked if they are cleared in the input field with the clear button #726
Unable to clear Select component's value #761

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 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

@ElDiddi
Copy link
Contributor Author

ElDiddi commented Nov 17, 2020

GitHub tease me, i haven't checked in the components/AntDesign.csproj

@ElderJames
Copy link
Member

You should rebase the master branch.

@ElDiddi
Copy link
Contributor Author

ElDiddi commented Nov 18, 2020

/preview

@github-actions
Copy link

github-actions bot commented Nov 18, 2020

@Seanxwy
Copy link
Contributor

Seanxwy commented Nov 19, 2020

@ElDiddi

After reading your writing method, I am a little confused about why TItemValue and TItem should be used at the same time, and SelectOption should also be used like this.I don't think it makes sense.You can do this entirely with a parameter, such as ValueType

Regarding the use of value types, I suggest

<Select @bind-Value="@_selectedValue" ValueType="int">
    <SelectOption Value=1 Label="Jack" />
    <SelectOption Value=2 Label="Lucy" />
    <SelectOption Value=3 Label="Yaoming" Disabled />
    <SelectOption Value=4 Label="Frieda" />	
</Select>

@ElDiddi
Copy link
Contributor Author

ElDiddi commented Nov 19, 2020

Hey @Yuanxw612,

in C# it is not possible to set a default value to a @typeparam (TypeScript: TValue = string) and in .Net 5 it is not possible to pass the @typeparam's down to child components. That is a planed feature in .Net 6 dotnet/aspnetcore#7268

If you wish we can discuss this in Discord.

@TimChen44
Copy link
Member

@ElDiddi
检查演示时发现一个问题。
A problem was found when checking the demo.

"自动分词"的Demo在复制带有”,“符号的文本后,组件并没有正确分词。
After copying the text with the symbol of "," in the Demo of "Automatic tokenization", the component did not segment correctly.

@ElderJames
Copy link
Member

Hey @ElDiddi , has #695 been fixed?

@ElderJames
Copy link
Member

@ElDiddi I think you may fixed #518 in this PR. Could you confirm?

@ElderJames
Copy link
Member

In demo multiple selection, the display order of the selected items have to be the order of selection, not the order of options.

currently:
image

expected:
image

@codecov-io
Copy link

Codecov Report

Merging #800 (4d30f07) into master (94d878e) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #800      +/-   ##
=========================================
- Coverage    3.67%   3.62%   -0.06%     
=========================================
  Files         394     398       +4     
  Lines       17469   17724     +255     
=========================================
  Hits          642     642              
- Misses      16827   17082     +255     
Impacted Files Coverage Δ
components/calendar/internal/CalendarHeader.razor 0.00% <0.00%> (ø)
components/core/Base/AntInputComponentBase.cs 0.00% <0.00%> (ø)
components/core/Component/overlay/Overlay.razor.cs 12.13% <0.00%> (-0.56%) ⬇️
components/core/JsInterop/JSInteropConstants.cs 7.69% <0.00%> (-0.21%) ⬇️
components/select/LabelTemplateItem.razor 0.00% <0.00%> (ø)
components/select/LabelTemplateItem.razor.cs 0.00% <0.00%> (ø)
components/select/Properties.cs 0.00% <ø> (ø)
components/select/Select.razor 0.00% <0.00%> (ø)
components/select/Select.razor.cs 0.00% <0.00%> (ø)
components/select/SelectLocale.cs 0.00% <ø> (ø)
... and 16 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 94d878e...4d30f07. Read the comment docs.

Copy link
Member

@ElderJames ElderJames left a comment

Choose a reason for hiding this comment

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

Merge this PR first and fix the rest through others.

@ElderJames ElderJames merged commit 5a49129 into ant-design-blazor:master Nov 27, 2020
ElderJames added a commit that referenced this pull request Apr 23, 2022
* select-rewritten

select-rewritten

* modul: select - update 1

* select-update2

* select-update3

* select-update 4

* feat: append label tag & auto tokenization

* fix: conflict in drpdown demo

Co-authored-by: Lars Diederich <diederich@evodata.de>
Co-authored-by: ElderJames <shunjiey@hotmail.com>
ElderJames added a commit that referenced this pull request Apr 30, 2022
* select-rewritten

select-rewritten

* modul: select - update 1

* select-update2

* select-update3

* select-update 4

* feat: append label tag & auto tokenization

* fix: conflict in drpdown demo

Co-authored-by: Lars Diederich <diederich@evodata.de>
Co-authored-by: ElderJames <shunjiey@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment