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: add toggle callback to Ellipsis component #6576
feat: add toggle callback to Ellipsis component #6576
Conversation
PR preview has been successfully built and deployed to https://antd-mobile-preview-pr-6576.surge.sh |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6576 +/- ##
=======================================
Coverage 92.34% 92.34%
=======================================
Files 318 318
Lines 6910 6912 +2
Branches 1729 1731 +2
=======================================
+ Hits 6381 6383 +2
Misses 493 493
Partials 36 36 ☔ View full report in Codecov by Sentry. |
这里我只是先发出来讨论一下,感觉用toggle这个名字不太好
|
@@ -21,6 +21,10 @@ export type EllipsisProps = { | |||
stopPropagationForActionButtons?: PropagationEvent[] | |||
onContentClick?: (e: React.MouseEvent<HTMLDivElement, MouseEvent>) => void | |||
defaultExpanded?: boolean | |||
toggle?: ( |
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.
Could we use onEllipsis
instead. It's same as the antd API:
https://ant.design/components/typography-cn#api
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.
Indeed, the design of onEllipsis in antd is more reasonable. The correct logic is to trigger onEllipsis based on whether the text is ellipsized or not. I'll continue working on it.
Why close? |
1 similar comment
Why close? |
close #6570