-
Notifications
You must be signed in to change notification settings - Fork 26
fix: use numeric input for lineHeight tokens #223
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
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @Chralay, 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! 此拉取请求旨在解决一个关键的 TypeScript 类型不兼容问题,该问题发生在从主题编辑器导出配置时, Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
📝 WalkthroughWalkthrough该变更在 TokenContent.tsx 中将非色彩种子输入的处理分为两个独立分支:保留 boxShadow 开头的 tokenName 使用 TextArea 渲染,新增 lineHeight 开头的 tokenName 使用 InputNumber 组件渲染。同时添加了 InputNumber 的显式导入。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/token-panel-pro/TokenContent.tsx`:
- Around line 398-406: The InputNumber for lineHeight currently falls back to 0
when newValue is nullish which can hide text; in the token rendering branch
where tokenName.startsWith('lineHeight') use InputNumber props to enforce a
sensible minimum (e.g., min={1}) and change the onChange fallback from 0 to a
safer default (e.g., 1) by passing handleChange(newValue ?? 1); also add/update
the seedRange for the 'lineHeight' token in the same token definition area
(similar to fontSize/borderRadius) to provide a consistent allowed range for
generated values.
| {tokenName.startsWith('lineHeight') && ( | ||
| <div> | ||
| <InputNumber<number> | ||
| style={{ minWidth: 200 }} | ||
| value={tokenValue} | ||
| onChange={(newValue) => handleChange(newValue ?? 0)} | ||
| /> | ||
| </div> | ||
| )} |
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.
清除输入时回退到 0 可能导致问题
当用户清空输入框时,newValue 为 null/undefined,代码会将其设为 0。但 lineHeight: 0 会导致文本不可见或排版异常。
建议添加 min 约束,并使用更合理的回退值:
🔧 建议的修复
{tokenName.startsWith('lineHeight') && (
<div>
<InputNumber<number>
style={{ minWidth: 200 }}
value={tokenValue}
- onChange={(newValue) => handleChange(newValue ?? 0)}
+ onChange={(newValue) => handleChange(newValue ?? 1)}
+ min={1}
+ step={0.1}
/>
</div>
)}另外,可考虑像其他数值类型 token(如 fontSize、borderRadius)一样,在 seedRange 中为 lineHeight 定义合理的取值范围,以保持一致性。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {tokenName.startsWith('lineHeight') && ( | |
| <div> | |
| <InputNumber<number> | |
| style={{ minWidth: 200 }} | |
| value={tokenValue} | |
| onChange={(newValue) => handleChange(newValue ?? 0)} | |
| /> | |
| </div> | |
| )} | |
| {tokenName.startsWith('lineHeight') && ( | |
| <div> | |
| <InputNumber<number> | |
| style={{ minWidth: 200 }} | |
| value={tokenValue} | |
| onChange={(newValue) => handleChange(newValue ?? 1)} | |
| min={1} | |
| step={0.1} | |
| /> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@src/token-panel-pro/TokenContent.tsx` around lines 398 - 406, The InputNumber
for lineHeight currently falls back to 0 when newValue is nullish which can hide
text; in the token rendering branch where tokenName.startsWith('lineHeight') use
InputNumber props to enforce a sensible minimum (e.g., min={1}) and change the
onChange fallback from 0 to a safer default (e.g., 1) by passing
handleChange(newValue ?? 1); also add/update the seedRange for the 'lineHeight'
token in the same token definition area (similar to fontSize/borderRadius) to
provide a consistent allowed range for generated values.
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 修复了 lineHeight token 在主题编辑器中导出为字符串,导致 TypeScript 类型不兼容的问题。通过将输入控件从 Input.TextArea 更换为 InputNumber,确保了 lineHeight 的值为数字类型。整体修改是正确的,并且解决了描述中的问题。我只提出了一点小建议,以增强 InputNumber 组件的健壮性和易用性。
| <InputNumber<number> | ||
| style={{ minWidth: 200 }} | ||
| value={tokenValue} | ||
| onChange={(newValue) => handleChange(newValue ?? 0)} | ||
| /> |
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.
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.
用户输入负值时是用户的事情啊,为啥要防止呢?
用[主题编辑器]修改行高之后,导出主题配置theme.json,将它放到

<ConfigProvider>后会出现TS报错:"token.lineHeight" 的类型不兼容。类型“string”不可与类型“number”进行比较。产生TS报错的原因是theme.json里lineHeight为string类型与ThemeConfig类型要求的number不一致。
当前PR将
lineHeight*字段改为InputNumber,确保导出配置时是number类型。Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.