-
Notifications
You must be signed in to change notification settings - Fork 220
fix: 字体变大时行高自适应变高 #3230
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
fix: 字体变大时行高自适应变高 #3230
Conversation
|
你好 @Alexzjt,非常感谢你的贡献. Hello, @Alexzjt, Thanks for your contribution. In order to make the code more robust, please add the corresponding unit tests, and update the docs if there are API changes. |
Summary of ChangesHello @Alexzjt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此 PR 旨在解决当字体大小增加时,单元格行高无法正确自适应的问题。通过调整核心的行高计算逻辑,并扩展了枢轴表和表格中行高自适应的触发条件,确保在字体变大时,单元格能够自动调整其高度以容纳内容。此外,还引入了一个统一的默认字体大小常量,以提高主题配置的一致性。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
您好,感谢您的贡献。本次 PR 旨在解决字体变大时行高无法自适应的问题,整体修改思路清晰,实现合理。主要改动包括:在 base-facet.ts 中移除了 isMultiLineText() 的检查,使得单行大字体文本也能正确触发行高自适应;在 pivot-facet.ts 和 table-facet.ts 中更新了启用高度自适应的逻辑,将字体大小也作为判断条件之一;在 theme/index.ts 中引入了 DEFAULT_FONTSIZE 常量来替代硬编码的字号 12,这是一个很好的重构,提高了代码的可维护性。我有一个小建议,希望能在 pivot-facet.ts 中也使用新引入的 DEFAULT_FONTSIZE 常量,以保持代码风格的统一。具体请看我的评论。
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #3230 +/- ##
==========================================
+ Coverage 75.77% 79.67% +3.90%
==========================================
Files 257 217 -40
Lines 11994 11985 -9
Branches 2464 2687 +223
==========================================
+ Hits 9088 9549 +461
+ Misses 1398 811 -587
- Partials 1508 1625 +117 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Size Change: +144 B (+0.02%) Total Size: 628 kB
ℹ️ View Unchanged
|
|
/gemini review |
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.
Code Review
本次 PR 旨在修复当设置大字体时单元格行高无法自适应的问题,整体思路正确,通过修改触发高度自适应的条件来兼容字体大小的变化,并添加了相应的测试用例。然而,审查中发现了一些可以改进的地方:
- 新添加的测试用例中存在代码重复。
- 在
base-facet.ts中,访问主题属性时的可选链使用不一致。 - 在
pivot-facet.ts和table-facet.ts中,检查字体大小的逻辑似乎不完整或存在错误,这可能导致在某些情况下高度自适应不会被正确触发。
建议修复以上问题,以确保代码的健壮性和可维护性。
|
/gemini review |
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.
Code Review
这个 PR 旨在解决当字体变大时单元格行高不能自适应变高的问题。通过在主题字体大于默认值时启用自适应高度计算,本次修改成功地解决了这个问题。修改涉及 base-facet、pivot-facet 和 table-facet,覆盖了不同的单元格类型和表格模式。同时,新增的测试用例确保了修复的正确性。引入 DEFAULT_FONTSIZE 常量也是一个很好的重构。总体而言,代码修改得很好,我有一个关于代码完整性的建议。
|
🎉 This PR is included in version @antv/s2-v2.4.9 🎉 The release is available on: Your semantic-release bot 📦🚀 |
👀 PR includes
✨ Feature
🎨 Enhance
🐛 Bugfix
🔧 Chore
📝 Description
🖼️ Screenshot
🔗 Related issue link
🔍 Self-Check before the merge