Conversation
📝 WalkthroughWalkthrough新增 Watermark 组件及其类型、样式、文档、演示与测试,组件实现支持文本或图片水印平铺、布局测量和前景/背景渲染,并将该组件导出到入口文件与类型导出中。 Changes
Sequence Diagram(s)(无) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 庆祝诗
🚥 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 |
Summary of ChangesHello @lqr131115, 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 delivers a new 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new "Watermark" component. A potential Denial of Service (DoS) vulnerability was identified in the rendering logic where an unbounded number of watermark items could be created if small dimensions are provided. This is mitigated by implementing the maxCount prop and enforcing minimum dimension values in the calculation. Additionally, there's a correctness bug in how the number of watermark items is calculated, which could result in incomplete coverage. Improvements are also suggested for better theme integration by removing hardcoded styles, addressing inconsistencies in the demo code and documentation, and enhancing unit tests to cover actual watermark rendering.
| import rnDemoTest from '../../../tests/shared/demoTest' | ||
|
|
||
| rnDemoTest('watermark') |
There was a problem hiding this comment.
The current test setup using react-test-renderer does not test the rendering of the watermarks themselves. The Watermark component relies on the onLayout event to determine its size and render the watermark elements, but react-test-renderer does not trigger onLayout. As a result, the generated snapshot (demo.test.js.snap) only contains the children of the Watermark component, and the watermark layer is missing.
To properly test this component, you should consider using a testing library that supports firing layout events, such as @testing-library/react-native. This would allow you to simulate a layout event and assert that the watermarks are rendered correctly.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@components/watermark/demo/basic.md`:
- Around line 10-17: 在导入行中移除未使用的 React hook 引入:删除 useState 从 import React, {
useState } ... 中,因为组件是类组件 WatermarkExample 并未使用该 hook;保持其他导入如
Dimensions、StyleSheet、View、Button、Watermark、WhiteSpace 不变以避免破坏示例。
In `@components/watermark/index.en-US.md`:
- Around line 17-30: The docs table is missing the styles prop from
WatermarkProps; add a new row describing "styles" as the override entry for
per-part styling (type `Partial<WatermarkStyle>`), with a short description like
"Custom style overrides for watermark parts" and no default; reference the
WatermarkProps declaration and WatermarkStyle type so users can locate the shape
to override when editing the component docs.
In `@components/watermark/index.tsx`:
- Around line 38-42: The current computation of _width, _height and count can
produce Infinity/NaN if width or height are zero/negative; update the watermark
sizing logic (variables _width, _height and count) to guard against
zero/negative values by clamping _width and _height to a safe minimum (e.g.,
Math.max(1, width + gapX * 2) / Math.max(1, height + gapY * 2)) or by returning
early with count = 0 when width or height are invalid; ensure all uses of count
and layout.width/layout.height handle the guarded values to avoid
division-by-zero.
In `@components/watermark/index.zh-CN.md`:
- Around line 18-31: The API table is missing the styles prop from
WatermarkProps; add a new row describing "styles" with type
"Partial<WatermarkStyle>" and a short description like
"自定义水印各元素样式(可选),会合并到默认样式中" and indicate default as "-" or "undefined"; reference
the WatermarkProps and WatermarkStyle symbols so readers can find the
interface/type and ensure the docs state it's optional and merged with default
styles.
🧹 Nitpick comments (5)
components/watermark/PropsType.tsx (1)
5-18: 建议将style改为StyleProp<ViewStyle>以匹配 RN 习惯
当前ViewStyle不支持样式数组或注册样式对象,使用StyleProp<ViewStyle>更符合 RN 组件 API。♻️ 建议修改
export interface WatermarkProps { content?: string | string[] contentStyle?: StyleProp<TextStyle> image?: string | React.ReactNode imageStyle?: StyleProp<ImageStyle> width?: number height?: number gapX?: number gapY?: number rotate?: number foreground?: boolean children?: React.ReactNode - style?: ViewStyle + style?: StyleProp<ViewStyle> styles?: Partial<WatermarkStyle> }rn-kitchen-sink/demoList.js (1)
268-273: 考虑使用独特的图标。Watermark 条目的图标与 Card 组件使用了相同的图标 (
daARhPjKcxlSuuZ.png)。建议为 Watermark 提供一个独特的图标以便于区分。components/watermark/demo/basic.tsx (2)
73-79: 建议使用函数式 setState 来避免潜在的状态竞争。当基于前一个状态值更新状态时,使用函数式
setState可以确保获取最新的状态值。♻️ 建议修改
<Button type="primary" onPress={() => { - this.setState({ foreground: !this.state.foreground }) + this.setState((prevState) => ({ foreground: !prevState.foreground })) }}> {this.state.foreground ? '前景层' : '背景层'} </Button>
7-14: 考虑为 state 添加类型定义。使用
any类型会失去 TypeScript 的类型检查优势。建议为 state 定义明确的接口。♻️ 建议修改
+interface WatermarkExampleState { + props: Record<string, any> + foreground: boolean +} + -export default class WatermarkExample extends React.Component<any, any> { +export default class WatermarkExample extends React.Component<any, WatermarkExampleState> { constructor(props: any) { super(props) this.state = { props: {}, foreground: false, } }components/watermark/index.tsx (1)
40-42: 建议使用Math.floor替代parseInt(String(...), 10)。当前写法较为冗长,可以简化为更直观的数学运算。
♻️ 建议修改
const count = - parseInt(String(layout.width / _width), 10) * - parseInt(String(layout.height / _height), 10) + Math.floor(layout.width / _width) * Math.floor(layout.height / _height)
|
TODO: use canvas !!! 😢 2026-01-28.21.31.33.mov |
|
👍 |
|
建议参考 |
First of all, thank you for your contribution! :-)
Please makes sure that these checkboxes are checked before submitting your PR, thank you!
npm run lintand fix those errors before submitting in order to keep consistent code style.Extra checklist:
if isBugFix :
elif isNewFeature :
Summary by CodeRabbit
发布说明
新功能
文档
演示
测试
✏️ Tip: You can customize this high-level summary in your review settings.