refactor(fixednav): 优化组件徽标的实现方式,解决原有不对称问题#3061
Conversation
Walkthrough这次更改主要针对 FixedNav 组件中列表项的渲染逻辑以及样式调整。SCSS 文件中移除了 Changes
Sequence Diagram(s)sequenceDiagram
participant FN as FixedNav 组件
participant RL as renderListItem 方法
participant BG as Badge 组件
participant UI as UI 显示
FN->>+RL: 调用 renderListItem(item, index)
alt item 包含 num 属性
RL->>+BG: 包装列表项，传递 num 属性
BG-->>-UI: 渲染带徽标的列表项
else
RL-->>UI: 直接渲染列表项
end
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
action:review
irisSong
requested review from
oasis-cloud and
xiaoyatong
and removed request for
oasis-cloud
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3061 +/- ##
==========================================
Coverage 86.06% 86.07%
==========================================
Files 275 275
Lines 18234 18243 +9
Branches 2751 2754 +3
==========================================
+ Hits 15694 15703 +9
Misses 2535 2535
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/fixednav/fixednav.scss(0 hunks)src/packages/fixednav/fixednav.taro.tsx(3 hunks)src/packages/fixednav/fixednav.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- src/packages/fixednav/fixednav.scss
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/fixednav/fixednav.taro.tsx
[error] 101-101: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 105-105: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/packages/fixednav/fixednav.tsx
[error] 102-102: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 106-106: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🪛 GitHub Check: codecov/patch
src/packages/fixednav/fixednav.tsx
[warning] 70-70: src/packages/fixednav/fixednav.tsx#L70
Added line #L70 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/packages/fixednav/fixednav.taro.tsx (2)
9-9: 良好的组件依赖引入添加 Badge 组件的导入很合适，这有助于实现带有数字标记的列表项。
62-77: 代码结构优化良好将列表项渲染逻辑提取到独立函数中是很好的重构，提高了代码的可读性和可维护性。
src/packages/fixednav/fixednav.tsx (3)
8-8: 良好的组件依赖引入添加 Badge 组件的导入很合适，这有助于实现带有数字标记的列表项。
62-77: 代码结构优化良好将列表项渲染逻辑提取到独立函数中是很好的重构，提高了代码的可读性和可维护性。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-70: src/packages/fixednav/fixednav.tsx#L70
Added line #L70 was not covered by tests
69-70:Details
❓ Verification inconclusive
增加测试覆盖率
新添加的 icon 渲染逻辑未被测试覆盖，建议添加相应测试用例。
🏁 Script executed:
#!/bin/bash # 检查是否存在针对 icon 渲染逻辑的测试 rg -A 3 "icon.*isValidElement|isValidElement.*icon" src/packages/fixednav/__tests__Length of output: 85
建议补充针对 icon 渲染逻辑的单元测试
在src/packages/fixednav/fixednav.tsx中新增的 icon 渲染逻辑，目前在src/packages/fixednav/__tests__中未发现对应的测试用例。请补充相应的单元测试，以确保该逻辑在未来的修改中不被误破坏。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 70-70: src/packages/fixednav/fixednav.tsx#L70
Added line #L70 was not covered by tests
| <> | ||
| {item.num ? ( | ||
| <Badge value={item.num} top={8} right={6}> | ||
| {renderListItem(item, index)} | ||
| </Badge> | ||
| ) : ( | ||
| <img src={item.icon} alt="" /> | ||
| <>{renderListItem(item, index)}</> | ||
| )} | ||
| <View className={`${classPrefix}-list-text`}> | ||
| {item.text} | ||
| </View> | ||
| {item.num && <View className="b">{item.num}</View>} | ||
| </View> | ||
| </> | ||
| ) |
There was a problem hiding this comment.
修复 React Fragment 中缺少 key 属性的问题
在使用列表渲染时，每个 React Fragment 都需要添加 key 属性。
建议使用以下修复方案：
return (
- <>
+ <React.Fragment key={item.id || index}>
{item.num ? (
- <Badge value={item.num} top={8} right={6}>
+ <Badge value={item.num} top={8} right={6} key={`badge-${item.id || index}`}>
{renderListItem(item, index)}
</Badge>
) : (
- <>{renderListItem(item, index)}</>
+ <React.Fragment key={`item-${item.id || index}`}>{renderListItem(item, index)}</React.Fragment>
)}
- </>
+ </React.Fragment>
)📝 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.
| <> | |
| {item.num ? ( | |
| <Badge value={item.num} top={8} right={6}> | |
| {renderListItem(item, index)} | |
| </Badge> | |
| ) : ( | |
| <img src={item.icon} alt="" /> | |
| <>{renderListItem(item, index)}</> | |
| )} | |
| <View className={`${classPrefix}-list-text`}> | |
| {item.text} | |
| </View> | |
| {item.num && <View className="b">{item.num}</View>} | |
| </View> | |
| </> | |
| ) | |
| return ( | |
| <React.Fragment key={item.id || index}> | |
| {item.num ? ( | |
| <Badge value={item.num} top={8} right={6} key={`badge-${item.id || index}`}> | |
| {renderListItem(item, index)} | |
| </Badge> | |
| ) : ( | |
| <React.Fragment key={`item-${item.id || index}`}>{renderListItem(item, index)}</React.Fragment> | |
| )} | |
| </React.Fragment> | |
| ) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 101-101: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 105-105: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| <> | ||
| {item.num ? ( | ||
| <Badge value={item.num} top={8} right={6}> | ||
| {renderListItem(item, index)} | ||
| </Badge> | ||
| ) : ( | ||
| <img src={item.icon} alt="" /> | ||
| <>{renderListItem(item, index)}</> | ||
| )} | ||
| <div className={`${classPrefix}-list-text`}>{item.text}</div> | ||
| {item.num && <div className="b">{item.num}</div>} | ||
| </div> | ||
| </> |
There was a problem hiding this comment.
修复 React Fragment 中缺少 key 属性的问题
在使用列表渲染时，每个 React Fragment 都需要添加 key 属性。
建议使用以下修复方案：
return (
- <>
+ <React.Fragment key={item.id || index}>
{item.num ? (
- <Badge value={item.num} top={8} right={6}>
+ <Badge value={item.num} top={8} right={6} key={`badge-${item.id || index}`}>
{renderListItem(item, index)}
</Badge>
) : (
- <>{renderListItem(item, index)}</>
+ <React.Fragment key={`item-${item.id || index}`}>{renderListItem(item, index)}</React.Fragment>
)}
- </>
+ </React.Fragment>
)📝 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.
| <> | |
| {item.num ? ( | |
| <Badge value={item.num} top={8} right={6}> | |
| {renderListItem(item, index)} | |
| </Badge> | |
| ) : ( | |
| <img src={item.icon} alt="" /> | |
| <>{renderListItem(item, index)}</> | |
| )} | |
| <div className={`${classPrefix}-list-text`}>{item.text}</div> | |
| {item.num && <div className="b">{item.num}</div>} | |
| </div> | |
| </> | |
| return ( | |
| <React.Fragment key={item.id || index}> | |
| {item.num ? ( | |
| <Badge value={item.num} top={8} right={6} key={`badge-${item.id || index}`}> | |
| {renderListItem(item, index)} | |
| </Badge> | |
| ) : ( | |
| <React.Fragment key={`item-${item.id || index}`}> | |
| {renderListItem(item, index)} | |
| </React.Fragment> | |
| )} | |
| </React.Fragment> | |
| ) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 102-102: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 106-106: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
2 participants
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit