ng-zorro-antd icon indicating copy to clipboard operation
ng-zorro-antd copied to clipboard

fix(module:tree): tree select search slow in virtual mode

Open jpoyard opened this issue 2 years ago • 8 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [X] The commit message follows our guidelines: https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/CONTRIBUTING.md#commit
  • [X] Tests for the changes have been added (for bug fixes / features)
  • [X] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[X] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: https://github.com/NG-ZORRO/ng-zorro-antd/issues/5873

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

For nz-tree component when nzVirtualHeight is defined and nzHideUnMatched=true then we should remove node with canHide=true from nzFlattenNodes In showcase website, for tree select component, i put nzHideUnMatched=true for virtual scroll sample

jpoyard avatar Apr 25 '22 08:04 jpoyard

This preview will be available after the AzureCI is passed.

zorro-bot[bot] avatar Apr 25 '22 08:04 zorro-bot[bot]

Codecov Report

Merging #7385 (ffb9474) into master (ea1138b) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7385      +/-   ##
==========================================
+ Coverage   91.74%   91.76%   +0.01%     
==========================================
  Files         502      502              
  Lines       16700    16702       +2     
  Branches     2604     2605       +1     
==========================================
+ Hits        15321    15326       +5     
+ Misses       1048     1046       -2     
+ Partials      331      330       -1     
Impacted Files Coverage Δ
components/tree/tree.component.ts 94.41% <100.00%> (+0.06%) :arrow_up:
components/tabs/tab-nav-bar.component.ts 87.94% <0.00%> (+0.70%) :arrow_up:
components/core/util/text-measure.ts 92.98% <0.00%> (+0.87%) :arrow_up:

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 ea1138b...ffb9474. Read the comment docs.

codecov[bot] avatar Apr 25 '22 08:04 codecov[bot]

This question bothered me for a long time since version 11. I solved it by this way temporarily https://github.com/NG-ZORRO/ng-zorro-antd/issues/6426#issuecomment-1006769898 . But when I upgraded to version 13, I just cannot find the same file and it didnt work fine anymore. I really hope this fix can be released as soon as possilble.

Dylan0405 avatar Jun 24 '22 02:06 Dylan0405

This question bothered me for a long time since version 11. I solved it by this way temporarily #6426 (comment) . But when I upgraded to version 13, I just cannot find the same file and it didnt work fine anymore. I really hope this fix can be released as soon as possilble.

I just updated to v13 and cant find the files either to change this, i tried updating the mjs files but the changes dont reflect. I am guessing the library doesnt use the angular compatibility compiler anymore. Anyone know how to modify this without forking it? @Dylan0405 did you find away around it? Thanks

whittssg avatar Jun 30 '22 17:06 whittssg

@whittssg Not really. I also changed the codes both in file 'node_modules\ng-zorro-antd\fesm2015\ng-zorro-antd-tree.mjs' and 'node_modules\ng-zorro-antd\fesm2020\ng-zorro-antd-tree.mjs' as metioned before. It works if part of my build config in angular.json is {"buildOptimizer": true, "optimization": true, "vendorChunk": false, "extractLicenses": true, "sourceMap": false, "namedChunks": false} ,but if the config is like follows: { "buildOptimizer": false, "optimization": false, "vendorChunk": true, "extractLicenses": false, "sourceMap": true, "namedChunks": true } then it won't work well. I don't know which field leads to the differences.

Dylan0405 avatar Jul 02 '22 06:07 Dylan0405

@jpoyard I think there might be a small bug in this PR. If you go to the preview and scroll down to the virtual scroll demo, type a few characters and then delete them you end up with a blank list. It should revert to the full list once there are no characters in the search box (well it did before).

Maybe adding && this.nzSearchValue after this.nzHideUnMatched would work

Thanks

whittssg avatar Jul 04 '22 14:07 whittssg

@whittssg Not really. I also changed the codes both in file 'node_modules\ng-zorro-antd\fesm2015\ng-zorro-antd-tree.mjs' and 'node_modules\ng-zorro-antd\fesm2020\ng-zorro-antd-tree.mjs' as metioned before. It works if part of my build config in angular.json is {"buildOptimizer": true, "optimization": true, "vendorChunk": false, "extractLicenses": true, "sourceMap": false, "namedChunks": false} ,but if the config is like follows: { "buildOptimizer": false, "optimization": false, "vendorChunk": true, "extractLicenses": false, "sourceMap": true, "namedChunks": true } then it won't work well. I don't know which field leads to the differences.

I believe its just the new cache in angular 13.. delete the cache folder under .angular and it should pick up the changes

whittssg avatar Jul 04 '22 16:07 whittssg

@jpoyard I think there might be a small bug in this PR. If you go to the preview and scroll down to the virtual scroll demo, type a few characters and then delete them you end up with a blank list. It should revert to the full list once there are no characters in the search box (well it did before).

Maybe adding && this.nzSearchValue after this.nzHideUnMatched would work

Thanks

@whittssg, i added && this.nzSearchValue?.lengt > 0

jpoyard avatar Jul 05 '22 05:07 jpoyard

Has this been merged yet? Thanks

whittssg avatar Oct 27 '22 13:10 whittssg

@whittssg 这个好像还没合并,朋友,你找到其他解决方案没呢?

Has this been merged yet? Thanks

Dylan0405 avatar Nov 24 '22 03:11 Dylan0405