-
Notifications
You must be signed in to change notification settings - Fork 61
fix: resize style should work #305
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
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough新增一个名为 Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as 使用方组件
participant Mentions as Mentions
participant Memo as mergedStyles (memo)
participant TextArea as TextArea
note right of Mentions#lightgreen: 渲染流程(变更后)
Consumer->>Mentions: 传入 props (style, styles, ...)
Mentions->>Memo: 计算 mergedStyles(合并 resize 到 styles.textarea)
Memo-->>Mentions: 返回 mergedStyles
Mentions->>TextArea: 通过 `styles={mergedStyles}` 传递样式
TextArea-->>Consumer: 渲染带合并样式的 textarea
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
Summary of ChangesHello @zombieJ, 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! This pull request addresses an issue where the 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #305 +/- ##
==========================================
+ Coverage 98.48% 98.51% +0.03%
==========================================
Files 8 8
Lines 264 270 +6
Branches 64 66 +2
==========================================
+ Hits 260 266 +6
Misses 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 修复了 resize 样式因为 style 属性覆盖而失效的问题,做得很好。通过 useMemo 合并 style 和 styles 属性中的 resize 样式,确保了正确的优先级和应用。代码逻辑清晰,并且添加了相应的单元测试来覆盖这个修复,保证了代码质量。我有一个关于 useMemo 用法的重构建议,可以让代码更简洁,依赖关系更清晰。
src/Mentions.tsx
Outdated
| const resizeStyle = styles?.textarea?.resize ?? style?.resize; | ||
| const mergedStyles = React.useMemo(() => { | ||
| const mergedTextareaStyle = { ...styles?.textarea }; | ||
|
|
||
| // Only add resize if it has a valid value, avoid setting undefined | ||
| if (resizeStyle !== undefined) { | ||
| mergedTextareaStyle.resize = resizeStyle; | ||
| } | ||
|
|
||
| return { | ||
| ...styles, | ||
| textarea: mergedTextareaStyle, | ||
| }; | ||
| }, [resizeStyle, styles]); |
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.
可以将 resizeStyle 的计算逻辑移到 useMemo 内部,这样可以使 useMemo 的依赖项更直接地反映其依赖的 props (style 和 styles),而不是派生状态 resizeStyle。这能让代码更清晰,也更容易维护。
| const resizeStyle = styles?.textarea?.resize ?? style?.resize; | |
| const mergedStyles = React.useMemo(() => { | |
| const mergedTextareaStyle = { ...styles?.textarea }; | |
| // Only add resize if it has a valid value, avoid setting undefined | |
| if (resizeStyle !== undefined) { | |
| mergedTextareaStyle.resize = resizeStyle; | |
| } | |
| return { | |
| ...styles, | |
| textarea: mergedTextareaStyle, | |
| }; | |
| }, [resizeStyle, styles]); | |
| const mergedStyles = React.useMemo(() => { | |
| const resizeStyle = styles?.textarea?.resize ?? style?.resize; | |
| const mergedTextareaStyle = { ...styles?.textarea }; | |
| // Only add resize if it has a valid value, avoid setting undefined | |
| if (resizeStyle !== undefined) { | |
| mergedTextareaStyle.resize = resizeStyle; | |
| } | |
| return { | |
| ...styles, | |
| textarea: mergedTextareaStyle, | |
| }; | |
| }, [style, styles]); |
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
🧹 Nitpick comments (1)
tests/Mentions.spec.tsx (1)
372-388: LGTM!测试覆盖良好新增的测试用例正确验证了 resize 样式可以通过
style属性和styles.textarea属性两种方式应用。可选建议:考虑添加一个测试用例来验证优先级行为(当同时提供
style.resize和styles.textarea.resize时,styles.textarea.resize应该具有更高优先级):it('should prioritize resize style from styles.textarea over style prop', () => { const { container } = renderMentions({ style: { resize: 'horizontal' }, styles: { textarea: { resize: 'vertical' } }, }); const textarea = container.querySelector('textarea'); expect(textarea).toHaveStyle({ resize: 'vertical' }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Mentions.tsx(2 hunks)tests/DropdownMenu.spec.tsx(1 hunks)tests/Mentions.spec.tsx(1 hunks)
🔇 Additional comments (2)
src/Mentions.tsx (1)
507-507: LGTM!将合并后的 styles 传递给 TextArea 组件的做法是正确的,确保了 resize 样式能够正确应用。
tests/DropdownMenu.spec.tsx (1)
2-4: LGTM!移除未使用的导入是良好的代码维护实践,清理后的导入声明更加简洁明了。
resize样式不生效,两者会冲突。因为 style 的优先级会更高一些导致覆盖。Summary by CodeRabbit
发版说明
重构
测试
杂项