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

fix(module:select): fix clearing behavior, reduce ValueChanged triggered, change value after datasource changes, follow the datasource element order #1886

Closed
wants to merge 1,037 commits into from

Conversation

anranruye
Copy link
Member

@anranruye anranruye commented Aug 28, 2021

🤔 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

When a Select component:
1. has DefaultActiveFirstOption or DefaultValue attribute
2. inital value is not null
Then clearing is not not working as expected. When the first time clear, the selected value is set to DefaultValue/first option value

动画11

code:

<Select DataSource="@_list"
        @bind-Value="@_selectedValue1"
        DefaultActiveFirstOption
        ValueName="@nameof(Person.Value)"
        LabelName="@nameof(Person.Name)"
        DisabledName="@nameof(Person.IsDisabled)"
        Style="width:120px"
        AllowClear
        OnSelectedItemChanged="OnSelectedItemChangedHandler">
</Select>

<Button OnClick="()=>_selectedValue1 = null">Set value to null</Button>
@code
{
    List<Person> _list;
    string _selectedValue1 = "lucy";
    class Person
    {
        public string Value { get; set; }
        public string Name { get; set; }
        public bool IsDisabled { get; set; }
    }

    protected override void OnInitialized()
    {
        _list = new List<Person>
        {
            new Person {Value = "jack", Name = "Jack"},
            new Person {Value = "lucy", Name = "Lucy"},
            new Person {Value = "disabled", Name = "Disabled", IsDisabled = true},
            new Person {Value = "yaoming", Name = "YaoMing"}
        };
    }

    private void OnSelectedItemChangedHandler(Person value)
    {
        Console.WriteLine($"selected: ${value?.Name}");
    }
}

  1. ValueChanged is triggered multiple times, which is unnecessary.
    动画111
    code:
<Select DataSource="@_list"
        TItemValue="int?" TItem="Person"
        Value="@_selectedValue1" 
        ValueChanged="@((v) => { Console.WriteLine("ValueChanged:" + v); _selectedValue1 = v; })"
        ValueName="@nameof(Person.Value)"
        LabelName="@nameof(Person.Name)"
        DisabledName="@nameof(Person.IsDisabled)"
        Style="width:120px"
        AllowClear
        DefaultActiveFirstOption
        OnSelectedItemChanged="OnSelectedItemChangedHandler">
</Select>

@_selectedValue1

@code
{
    List<Person> _list;
    int? _selectedValue1 = 1;
    class Person
    {
        public int? Value { get; set; }
        public string Name { get; set; }
        public bool IsDisabled { get; set; }
    }

    protected override void OnInitialized()
    {
        _list = new List<Person>
        {
            new Person {Value = 1, Name = "Jack"},
            new Person {Value = 2, Name = "Lucy"},
            new Person {Value = 3, Name = "Disabled", IsDisabled = true},
            new Person {Value = 4, Name = "YaoMing"}
        };
    }

    private void OnSelectedItemChangedHandler(Person value)
    {
        Console.WriteLine($"selected: ${value?.Name}");
    }
}


  1. the selected value is not changed when the selected option no longer exists after datasource changes
    动画112
    动画113

  1. After change the order of elements in the datasource, the list doesn't follow the new element order.
    动画115

code:

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

<Button OnClick="()=>_data = new List<int?> { 2, 3, 4, 5 }">2345</Button>

<Button OnClick="()=>_data = new List<int?> { 2, 1, 3, 4 }">2134</Button>

@code
{
    List<int?> _data = new List<int?> { 1, 2, 3, 4 };
    int? _value = 4;
}

📝 Changelog

Language Changelog
🇺🇸 English fix wrong behavior when clear the selection; reduce ValueChanged triggered, change value if the selected option no longer exists after datasource changes; after change the order of elements in the datasource, make the list follow the new element order.
🇨🇳 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

anddrzejb and others added 30 commits April 4, 2021 14:01
…er (ant-design-blazor#1263)

* refactor: support to use the same template for confirm and modal

* refactor: support to use the same template for drawer

* refactor: separate interface IOkCancelRef

* chore: modify EventUtil class summary

Co-authored-by: James Yeung <shunjiey@hotmail.com>
…esign-blazor#1250)

* feat(Alert): added alert loop component

* fix: move cmp into main

* feat: add parameters for looping text

* feat: add new messages loop

* fix: create new internal looptext cmp

* doc: add demo

* doc: add demo markdown

* doc: update alert api

* doc: update cn docs

* fix: add missing dependency

* fix: update param name

* impleement loop text with css

* fix the document

Co-authored-by: James Yeung <shunjiey@hotmail.com>
…azor#1310)

feat(module:table): add support for Display attribute

Display attribute is widely used to specify display text for entity properties.
Table component should get column names from Display attribute instances.

Closes ant-design-blazor#1278
* test: change folder structure and add new TestKit csproj for public testing of AntDesign-based applications

* docs: added CN and EN docs about TestKit

* fix doc translate

* change the directory structure

Co-authored-by: Patryk Grzelak <pgrzelak@mutate.app>
Co-authored-by: James Yeung <shunjiey@hotmail.com>
* fix(module: dropdown): default PlacementType is incorrect in RTL mode

* fix(module: datepicker): not correct in RTL mode

* fix(module: datepicker): styles is incorrect when switch to LTR from RTL

Co-authored-by: James Yeung <shunjiey@hotmail.com>
…n-blazor#1309)

* feat(module:select): allow overlay to match item width

* fix(module:select): resize selectbox on window resize

* docs(module:select): add new parameters

* fix(module:select): add DropdownMatchSelectWidth & DropdownMaxWidth

* fix(module:select): apply same layout as antD

apply same behavior as antD for backspace
split code to default/multiple&tags
always focus on input for search-able
handle overflow & ellipsis for to long items
rendering optimization for SelectContent

* fix(module:select): backspace functionality limit
* fix(module: table): ExpandableRow not working with ActionColumn ant-design-blazor#1284 (ant-design-blazor#1285)

* (fix) Table ExpandableRow not working with ActionColumn ant-design-blazor#1284

* Add Tree Button Expand on Action Columm.

Check the position of Selection Column when calculate the position for expand button on tree mode and expand mode.

* fix(module: rate): Value has priority over DefaultValue (ant-design-blazor#1303)

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* chore: add github actions for auto preview

* fix setup job

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* fix(module:rate): prevent reset to default value

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* feat(module: form): Add LabelTemplate for FormItem (ant-design-blazor#1293)

Co-authored-by: Leishi <lluo@octet.com>

* fix(module: table): fix DataIndex Column with incorrect sorting and filter (ant-design-blazor#1295)

* refactor(module: table): Unify the behavior of Field and DataIndex

* fix(module: table): Fix DataIndex Column doesn't refresh

* doc(module: table): add DataIndex column filter demo

* doc(module: table): fix Blazor table

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: datepicker): DatePicker DisabledDate works incorect (ant-design-blazor#1298)

* fix(module: datepicker): DatePicker DisabledDate works incorect

* test(module: dateHelperTests): modify test case

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: upload): Upload list update fix (ant-design-blazor#1301)

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* chore: add github actions for auto preview

* fix setup job

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* fix(module:upload): file list update

* fix(module:upload): typed args for file upload

* fix(module:upload): delete button remove from picture-card

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: textarea): default to empty string (ant-design-blazor#1305)

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* chore: add github actions for auto preview

* fix setup job

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* fix(module:textarea): add parameter DefaultToEmptyString

* fix(module:textarea): occasional crashes of js in firefox

* docs(module:input): added new param DefaultToEmptyString

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: grid): missing flex and wrap style (ant-design-blazor#1296)

* fix(module: grid): missing flex style

* remove the old md file

* fix demos

* fix build error

* fix no warp class

* docs: add changelog in document (ant-design-blazor#1306)

* fix(module: select): load icon for multiple mode & value reload on delay (ant-design-blazor#1307)

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* chore: add github actions for auto preview

* fix setup job

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* fix(module:select): add loading for multiple mode

* fix(module:select): selected items duplication for multiple mode when values changed outside the component

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: table): perf optimization & data source change issue (ant-design-blazor#1304)

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* chore: add github actions for auto preview

* fix setup job

* chore: add GitHub Actions for auto preview (ant-design-blazor#1205)

* fix(module:table): guard against false entries in _dataSourceCache

* fix(module:table): perf - double to single render on datasource change

* fix(module:table): perf optimization to avoid multiple renders

Co-authored-by: James Yeung <shunjiey@hotmail.com>

* fix(module: select): propagation on item remove (ant-design-blazor#1308)

* fix(module: table): fix table re-render when ScrollX is set (ant-design-blazor#1311)

* docs(module: table): fix RouterPagging demo (ant-design-blazor#1313)

* chagnelog 0.7.4 (ant-design-blazor#1321)

Co-authored-by: Magehernan <magehernan@gmail.com>
Co-authored-by: Andrzej Bakun <andrzej@neelyc.com.cy>
Co-authored-by: ldsenow <ldsenow@gmail.com>
Co-authored-by: Leishi <lluo@octet.com>
Co-authored-by: Zonciu Liang <zonciu@zonciu.com>
Co-authored-by: 笨木头 <musicvs@163.com>
…nt-design-blazor#1325)

* feat(module: pagination): add TotalBoundaryShowSizeChanger

* docs(module: pagination): update API

Co-authored-by: James Yeung <shunjiey@hotmail.com>
* feat(docs): change color dynamically

* docs: dynamic primary color changing

* fix file path

* delete the script
* tablefilter-moreoptions

* tablefilter-moreoptions

* tablefilter-moreoptions

* tablefilter-moreoptions

* tablefilter-moreoptions

* fix demo and docs

Co-authored-by: James Yeung <shunjiey@hotmail.com>
…ant-design-blazor#1328) (ant-design-blazor#1336)

* fix(module:result): modify status unsuccessfully after initialization(ant-design-blazor#1328)

* fix(module:result): determine whether the Icon is empty string(ant-design-blazor#1328)

* doc: add changeTheResult demo(ant-design-blazor#1328)

* delete some lines
…zor#1338)

* feat(module:select): add MaxTagTextLength, MaxTagCount & MaxTagPlaceHolder

* feat(module:select): responsive maxTagCount using window.resize

* docs(module:select): MaxTagCount functionality

* fix docs

Co-authored-by: ElderJames <shunjiey@hotmail.com>
…nt-design-blazor#1335)

* fix(module: affix): affix to the viewport (ant-design-blazor#1327)

* fix target to element

* rename to target selector

Co-authored-by: ElderJames <shunjiey@hotmail.com>
CAPCHIK and others added 6 commits August 28, 2021 03:07
…nt-design-blazor#1859)

* Setup breadcrumb dropdown

* Setup breadcrumb href

* fix dropdown style

* fix dropdown trigger class

* fix tests

Co-authored-by: James Yeung <shunjiey@hotmail.com>
…ign-blazor#1883)

Fixed an issue where a table would not automatically load after initialization when an ActionColumn was used.
…the trigger button (ant-design-blazor#1838)

* fix: second opening of focus in modal fails if DestroyOnClose is false

* fix: confirm cannot get focus element

* fix: set ConfirmAutoFocusButton is OK

* fix: module ImagePreview cannot close on second click

* fix: blur active element when comfirm focus element is disabled
@github-actions
Copy link

github-actions bot commented Aug 28, 2021

@codecov
Copy link

codecov bot commented Aug 28, 2021

Codecov Report

Merging #1886 (5487d3b) into master (ad2c045) will increase coverage by 23.69%.
The diff coverage is 54.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1886       +/-   ##
===========================================
+ Coverage    0.00%   23.69%   +23.69%     
===========================================
  Files         479      496       +17     
  Lines       31330    23360     -7970     
  Branches        0      122      +122     
===========================================
+ Hits            0     5535     +5535     
+ Misses      31330    17822    -13508     
- Partials        0        3        +3     
Impacted Files Coverage Δ
components/select/Select.razor.cs 45.00% <54.00%> (+45.00%) ⬆️
components/card/Card.razor 0.00% <0.00%> (ø)
components/form/Form.razor 0.00% <0.00%> (ø)
components/rate/Rate.razor 0.00% <0.00%> (ø)
components/spin/Spin.razor 0.00% <0.00%> (ø)
components/tree/Tree.razor 0.00% <0.00%> (ø)
components/menu/MenuLink.cs 0.00% <0.00%> (ø)
components/menu/MenuMode.cs 0.00% <0.00%> (ø)
components/steps/Step.razor 0.00% <0.00%> (ø)
components/affix/Affix.razor 0.00% <0.00%> (ø)
... and 439 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 ad2c045...5487d3b. Read the comment docs.

@anranruye anranruye changed the title fix(module:select): fix wrong behavior when clear the selection fix(module:select): fix clearing behavior, reduce ValueChanged triggered, change value after datasource changes Aug 29, 2021
@anranruye anranruye changed the title fix(module:select): fix clearing behavior, reduce ValueChanged triggered, change value after datasource changes fix(module:select): fix clearing behavior, reduce ValueChanged triggered, change value after datasource changes, follow the datasource element order Aug 29, 2021
@anranruye
Copy link
Member Author

anranruye commented Aug 29, 2021

To always follow the order of elements in the datasource, I clear the SelectOptionItems hashset and add the option items back one by one every time CreateDeleteSelectOptions() is called. I'm afraid this may affect performance when there is many select options. But I don't know how much impact this will have. @ElderJames @anddrzejb Do you think we should add a KeepDataSourceElementOrder property to control the open and close of this function?

@anddrzejb
Copy link
Member

I agree with you that with larger number of elements in the select, current approach may impact the performance. Exposing a parameter (especially primitive boolean) gives a flexibility in opposition to a very opinionated solution. I cannot really see any harm in adding the suggested parameter. I am therefore voting in favor of KeepDataSorceElementOrder. This probably should default to true and in the docs we should explain why setting this to false can improve performance when dealing with large changeable lists.

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.

None yet