Skip to content

perf(module:select): ability to pass key | nzKey to select option #8033

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

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

oleksandr-codefresh
Copy link
Contributor

@oleksandr-codefresh oleksandr-codefresh commented Jul 26, 2023

When 'object' is used as 'value' for select option re-rendering occurs each time data comes, to solve this added new 'key' param | 'nzKey' Input

PR Checklist

Please check if your PR fulfills the following requirements:

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
[x] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Under the hood nz-select uses value | nzValue as key for rendering optimizations. In case if value of 'object' type this doesn't work well.

Issue Number: #8034

What is the new behavior?

We use value as fallback and allow passing key | nzKey for this specific purpose

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@zorro-bot
Copy link

zorro-bot bot commented Jul 26, 2023

This preview will be available after the AzureCI is passed.

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #8033 (2785f5f) into master (544f308) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is 70.00%.

❗ Current head 2785f5f differs from pull request most recent head bc4cf48. Consider uploading reports for the commit bc4cf48 to get more accurate results

@@            Coverage Diff             @@
##           master    #8033      +/-   ##
==========================================
- Coverage   91.64%   91.64%   -0.01%     
==========================================
  Files         515      515              
  Lines       17637    17642       +5     
  Branches     2789     2792       +3     
==========================================
+ Hits        16164    16168       +4     
+ Misses       1175     1174       -1     
- Partials      298      300       +2     
Files Changed Coverage Δ
components/select/option.component.ts 100.00% <ø> (ø)
components/tree/tree-node.component.ts 92.43% <ø> (+0.49%) ⬆️
components/select/select.component.ts 92.09% <33.33%> (-0.51%) ⬇️
components/qr-code/qrcode.component.ts 90.69% <85.71%> (-1.62%) ⬇️

... and 1 file with indirect coverage changes

@oleksandr-codefresh
Copy link
Contributor Author

@vthinkxie pls review once you will be able, thx)

@oleksandr-codefresh
Copy link
Contributor Author

@hsuanxyz Hi, maybe you can review? thx)

@hsuanxyz hsuanxyz requested a review from simplejason July 28, 2023 05:56
@hsuanxyz
Copy link
Member

@simplejason check this plz.

@hsuanxyz
Copy link
Member

@oleksandr-codefresh can we change the commit type to perf

When object is used as key for select option re-rendering occurs, to solve this added new 'key' param | 'nzKey' Input
@oleksandr-codefresh oleksandr-codefresh changed the title fix(module:select): ability to pass key | nzKey to select option perf(module:select): ability to pass key | nzKey to select option Jul 28, 2023
@oleksandr-codefresh
Copy link
Contributor Author

@hsuanxyz yes, changed it

@oleksandr-codefresh
Copy link
Contributor Author

@simplejason @vthinkxie @hsuanxyz pls, review when it's possible for you 🙏

Copy link
Member

@hsuanxyz hsuanxyz left a comment

Choose a reason for hiding this comment

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

LGTM

@oleksandr-codefresh
Copy link
Contributor Author

@OriginRing Hi, could you pls add label 'PR: reviewed-approved' to pr?

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.

4 participants