Skip to content

Conversation

@yjq635
Copy link

@yjq635 yjq635 commented Dec 28, 2024

to specify search logic, such as matching against custom data

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

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

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@yjq635 yjq635 requested a review from hsuanxyz as a code owner December 28, 2024 10:21
@zorro-bot
Copy link

zorro-bot bot commented Dec 28, 2024

This preview will be available after the AzureCI is passed.

@codecov
Copy link

codecov bot commented Dec 28, 2024

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.89%. Comparing base (9607e11) to head (03db60c).
⚠️ Report is 224 commits behind head on master.

Files with missing lines Patch % Lines
components/tree-select/tree-select.component.ts 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8951      +/-   ##
==========================================
- Coverage   91.91%   91.89%   -0.02%     
==========================================
  Files         555      555              
  Lines       19708    19711       +3     
  Branches     2943     2945       +2     
==========================================
  Hits        18114    18114              
- Misses       1268     1269       +1     
- Partials      326      328       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

to specify search logic, such as matching against custom data NG-ZORRO#8707 NG-ZORRO#3478
@Nicoss54
Copy link
Collaborator

@Laffery i reviewd this PR, seems good on my side, jsut missing test

but other point this seems not existing in the original spec. What do you thing to tag it with blocked by antd design ?

@Nicoss54 Nicoss54 self-requested a review August 16, 2025 08:08
Copy link
Collaborator

@Nicoss54 Nicoss54 left a comment

Choose a reason for hiding this comment

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

Could you write the test for your development ?

@Laffery
Copy link
Collaborator

Laffery commented Aug 16, 2025

@Laffery i reviewd this PR, seems good on my side, jsut missing test

but other point this seems not existing in the original spec. What do you thing to tag it with blocked by antd design ?

I think the filterTreeNode property is what we should support --> https://ant.design/components/tree-select#api.

I think we should support thetreeNodeFilterProp property at the mean time, which will be used for filtering if filterTreeNode returns true.
There could be two breaking changes

  • the default value of treeNodeFilterProp is value, but ng-zorro uses title to filter nodes now
  • highlight on labels of search results are based on title, we have to remove it and highlight the whole title because ant design filters by value now

ng-zorro highlights the matched parts
image

antd highlights the whole title
image

In a word, I think this feature needs to change the original design, and I'm willing to work on it in v21

@Nicoss54
Copy link
Collaborator

@Laffery i reviewd this PR, seems good on my side, jsut missing test
but other point this seems not existing in the original spec. What do you thing to tag it with blocked by antd design ?

I think the filterTreeNode property is what we should support --> https://ant.design/components/tree-select#api.

I think we should support thetreeNodeFilterProp property at the mean time, which will be used for filtering if filterTreeNode returns true. There could be two breaking changes

  • the default value of treeNodeFilterProp is value, but ng-zorro uses title to filter nodes now
  • highlight on labels of search results are based on title, we have to remove it and highlight the whole title because ant design filters by value now

ng-zorro highlights the matched parts image

antd highlights the whole title image

In a word, I think this feature needs to change the original design, and I'm willing to work on it in v21

I agree, let's close this one so WDYT ?

@Laffery
Copy link
Collaborator

Laffery commented Aug 16, 2025

@Laffery i reviewd this PR, seems good on my side, jsut missing test
but other point this seems not existing in the original spec. What do you thing to tag it with blocked by antd design ?

I think the filterTreeNode property is what we should support --> https://ant.design/components/tree-select#api.
I think we should support thetreeNodeFilterProp property at the mean time, which will be used for filtering if filterTreeNode returns true. There could be two breaking changes

  • the default value of treeNodeFilterProp is value, but ng-zorro uses title to filter nodes now
  • highlight on labels of search results are based on title, we have to remove it and highlight the whole title because ant design filters by value now

ng-zorro highlights the matched parts image
antd highlights the whole title image
In a word, I think this feature needs to change the original design, and I'm willing to work on it in v21

I agree, let's close this one so WDYT ?

ok

@Laffery
Copy link
Collaborator

Laffery commented Aug 16, 2025

@yjq635 Thanks for your contribution, we are planning to implement this feature in v21 in antother way, which was mentioned in the comments above.
I will close this PR and please feel free to contribute on other features in our milestone #9263 if you like. Thanks again :)

@Laffery Laffery closed this Aug 16, 2025
@yjq635
Copy link
Author

yjq635 commented Sep 16, 2025

@yjq635 Thanks for your contribution, we are planning to implement this feature in v21 in antother way, which was mentioned in the comments above. I will close this PR and please feel free to contribute on other features in our milestone #9263 if you like. Thanks again :)

多谢,我是因为自己需要所以做了一个简单的输入函数,我也看到了搜索高亮问题,我自己因为菜 所以简单实现自己的需求,有更好的设计当然更好,谢谢。

Thank you. I made a simple input function because I needed it myself. I also saw the search highlighting issue. I simply implemented my own requirements because I'm not good at it. Of course, a better design would be even better. Thank you.

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.

3 participants