fix(module:checkboxGroup): option change does not lock checkboxes #1863

anddrzejb · 2021-08-23T09:04:44Z

🤔 This is a ...

🔗 Related issue link

💡 Background and solution

When Options is changing, an attached function that calculates CheckboxGroup current value should also be changed. This PR is forcing the change.

However

I see another issue here which is tightly connected to performance. Currently. because Options are OneOf, the reset of its value is done on basically any action done on the component (so clicking on a single checkbox in the group triggers the Options to be set again). This I think is due to the fact that Options is not a primitive type (as stated in #992). In this case though I do not think we can use a primitive type. What we could use is a class that will internally hold CheckboxOption array and will also internally monitor for the changes. My idea here is that such class could very quickly know if it changed and a simple comparison could be done to find out if a change really happen. Then, if it did, appropriate action could be called (like clearing current value of the options and reset the aforementioned function responsible for calculation).

📝 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

github-actions · 2021-08-23T09:05:00Z

codecov · 2021-08-23T09:15:05Z

Codecov Report

Merging #1863 (d04dd4f) into master (4738685) will increase coverage by 23.56%.
The diff coverage is 50.00%.

@@             Coverage Diff             @@
##           master    #1863       +/-   ##
===========================================
+ Coverage    0.00%   23.56%   +23.56%     
===========================================
  Files         478      495       +17     
  Lines       31192    23237     -7955     
  Branches        0      121      +121     
===========================================
+ Hits            0     5476     +5476     
+ Misses      31192    17758    -13434     
- Partials        0        3        +3

Impacted Files	Coverage Δ
components/checkbox/CheckboxGroup.razor.cs	`88.42% <50.00%> (+88.42%)`	⬆️
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 438 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 4738685...d04dd4f. Read the comment docs.

ElderJames · 2021-08-23T13:22:27Z

I see another issue here which is tightly connected to performance.

Yes, we have many components that have this issue. Maybe we can solve them later.

anddrzejb · 2021-08-23T14:16:25Z

I see another issue here which is tightly connected to performance.

Yes, we have many components that have this issue. Maybe we can solve them later.

We should aim at solving them before v1, as these changes will most of the times (if not always) be breaking changes.

ElderJames · 2021-08-23T14:54:58Z

Yes, you are right.

* fix(module: table): fix initial load and render (#1835) * fix(module: table): fix initial load and render * delete the useless method * chore: remove redundant semi-colons (#1812) Co-authored-by: James Yeung <shunjiey@hotmail.com> * fix(module: checkbox): checked state didn't follow the value change (#1841) * fix(module: menu): menu item click event should be triggered when menu selectable is false (#1843) * fix(module: tag): Tag component style parameters are not rendered (#1847) * fix(module: tag): Tag component style parameters are not rendered (#1846) * fix(module: tag): change call GetStyle in OnParameterSet Co-authored-by: haojiajun <haojiajun@vanelink.net> * fix(module: tree): SelectedNodeChanged would be fired twice twice (#1849) * fix(module: table): prevent propagation of expand button click events (#1850) * changelog 0.9.2 (#1851) * feat(module: upload): add method parameter (#1853) * add method parameter to upload component * update documentation * add test Co-authored-by: Noah Potash <noah.potash@outbreaklabs.com> * fix(module: table): avoid `OnChange` being called multiple times during initialization (#1855) * fix(module: table): avoid OnChange being called multiple times during initialisation * fix indent * fix(module: table): didn't refresh when using client side data source (#1858) * feat(module: table): add TheSameDateWith condition operator for DateTime column (#1856) * use date in datetime filter * fix spelling errors * Restore changes and add Date and Month FilterCompareOperator * hide time when filter date or month * Remove the month FilterCompareOperator * add locale data Co-authored-by: James Yeung <shunjiey@hotmail.com> * fix(module: checkbox): option change does not lock checkboxes (#1863) * fix(module: table): ignore milliseconds when applying the datetime filter (#1864) * remove milliseconds when filtering * remove the milliseconds when set the datetime filter value Co-authored-by: James Yeung <shunjiey@hotmail.com> * fix(module: input-number): default value binding (#1871) * fix(module: input-number): default value binding * fix default value * fix(module: autocomplete): overlay is showing for AutoCompleteSearch (#1860) * fix(module:autocomplete): overlay is showing for AutoCompleteSearch * translate comments * code cleanup Co-authored-by: ElderJames <shunjiey@hotmail.com> * fix(module: tag): rename `CheckedChange` to `CheckedChanged` (#1876) * Update Tag compliance with Blazor conventions Add the missing 'd' to CheckedChanged callback in order to properly support two ways binding on the Checked property * Fix references to renamed method in Tag component * Update tests for Tag Co-authored-by: Stefano Drussi <stefano.driussi@hotmail.it> Co-authored-by: James Yeung <shunjiey@hotmail.com> * feat(module: menu): Add IconTemplate for menu item (#1879) * docs: update CONTRIBUTING (#1882) * feat(module: breadcrumb): add `Href` parameter and overlay dropdown (#1859) * Setup breadcrumb dropdown * Setup breadcrumb href * fix dropdown style * fix dropdown trigger class * fix tests Co-authored-by: James Yeung <shunjiey@hotmail.com> * fix(module: table): call `StateHasChanged` in `InternalReload` (#1874) (#1875) * fix(module: table): not automatically load with ActionColumn (#1883) Fixed an issue where a table would not automatically load after initialization when an ActionColumn was used. * feat(module: table): add nested table demo (#1884) * fix(module: modal): auto focus the ok button and remove the focus of the trigger button (#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 * fix(module: table): fix the format issue of datetime samedate filter (#1889) * changelog 0.9.3 (#1890) Co-authored-by: Simon Cropp <simon.cropp@gmail.com> Co-authored-by: JohnHao421 <544106829@qq.com> Co-authored-by: haojiajun <haojiajun@vanelink.net> Co-authored-by: Noah Potash <digitalnugget@gmail.com> Co-authored-by: Noah Potash <noah.potash@outbreaklabs.com> Co-authored-by: SmallY <45689960+iamSmallY@users.noreply.github.com> Co-authored-by: Andrzej Bakun <andrzej@neelyc.com.cy> Co-authored-by: Stefano Driussi <stedri@gmail.com> Co-authored-by: Stefano Drussi <stefano.driussi@hotmail.it> Co-authored-by: Guyiming <guyiming2011@126.com> Co-authored-by: YongQuanRao <79885120+JamesGit-hash@users.noreply.github.com> Co-authored-by: Maksim <maksalmak@gmail.com> Co-authored-by: Nikolay Krondev <nikolaykrondev@users.noreply.github.com> Co-authored-by: zxyao <zxyao145@gmail.com> Co-authored-by: anranruye <54608128+anranruye@users.noreply.github.com>

fix(module:checkboxGroup): option change does not lock checkboxes

d04dd4f

pr-triage bot added the PR: unreviewed label Aug 23, 2021

ElderJames approved these changes Aug 23, 2021

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Aug 23, 2021

ElderJames merged commit d29b28c into ant-design-blazor:master Aug 23, 2021

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Aug 23, 2021

anddrzejb deleted the checkboxGroupOption branch August 23, 2021 14:16

ElderJames pushed a commit that referenced this pull request Apr 23, 2022

fix(module: checkbox): option change does not lock checkboxes (#1863)

cf13a2d

ElderJames pushed a commit that referenced this pull request Apr 30, 2022

fix(module: checkbox): option change does not lock checkboxes (#1863)

d858b7e

ElderJames pushed a commit that referenced this pull request Sep 6, 2022

fix(module: checkbox): option change does not lock checkboxes (#1863)

07f2e53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(module:checkboxGroup): option change does not lock checkboxes #1863

fix(module:checkboxGroup): option change does not lock checkboxes #1863

anddrzejb commented Aug 23, 2021

github-actions bot commented Aug 23, 2021 •

edited

codecov bot commented Aug 23, 2021

ElderJames commented Aug 23, 2021

anddrzejb commented Aug 23, 2021

ElderJames commented Aug 23, 2021

fix(module:checkboxGroup): option change does not lock checkboxes #1863

fix(module:checkboxGroup): option change does not lock checkboxes #1863

Conversation

anddrzejb commented Aug 23, 2021

🤔 This is a ...

🔗 Related issue link

💡 Background and solution

However

📝 Changelog

☑️ Self Check before Merge

github-actions bot commented Aug 23, 2021 • edited

codecov bot commented Aug 23, 2021

Codecov Report

ElderJames commented Aug 23, 2021

anddrzejb commented Aug 23, 2021

ElderJames commented Aug 23, 2021

github-actions bot commented Aug 23, 2021 •

edited