-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(module:popconfirm): Support custom icon. #2445
Conversation
My first PR, I would appreciate your guidance😊. |
Codecov Report
@@ Coverage Diff @@
## next #2445 +/- ##
==========================================
- Coverage 95.84% 95.83% -0.01%
==========================================
Files 487 487
Lines 12318 12322 +4
Branches 1663 1663
==========================================
+ Hits 11806 11809 +3
- Misses 152 153 +1
Partials 360 360
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
It's basically useful but here are some enhancements that should be made:
- Add test case.
- Should support
TemplateRef
. What if users want to use their own icons or two-tone icons? - Commit message should be in lower case and has no period
.
- In your commit message you should write
close #2196
in the second line, so that issue would be closed automatically
Oh, and the docs. |
Roger!Thank you for your guidance, I'll try again. |
HI @WOOOFEI we are going to merge next to master now, plz submit this pr to master now, closed now. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2196
What is the new behavior?
Does this PR introduce a breaking change?
Other information