-
Notifications
You must be signed in to change notification settings - Fork 268
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: tabs 代码简化,调整部分样式 #2949
base: next
Are you sure you want to change the base?
fix: tabs 代码简化,调整部分样式 #2949
Conversation
概述演练这个拉取请求(PR)主要涉及对 Tabs 和 TabPane 组件的多个文件进行修改。更改包括在多个演示文件中将第一个标签页的标题从 "Tab 1" 更改为 "Tab longitem",在 Taro 和 H5 环境下的演示文件都有此修改。同时,对 变更
可能相关的 PR
建议标签
建议审阅者irisSong 诗歌
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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: 2
🔭 Outside diff range comments (1)
src/packages/tabs/__test__/tabs.spec.tsx (1)
Line range hint
19-29
: 需要修复测试失败!测试期望元素具有 'nut-tabs-titles-scrollable' 类,但实际上只有 'nut-tabs-titles nut-tabs-titles-smile'。这可能是由于最近的样式调整导致的回归问题。
建议修复方案:
- <Tabs value="0" direction="horizontal" activeType="smile"> + <Tabs value="0" direction="horizontal" activeType="smile" scrollable>
🧹 Nitpick comments (3)
src/packages/tabs/demos/h5/demo16.tsx (1)
14-14
: 建议改进标签页内容和无障碍性建议:
- 标签页的内容应该是有意义的,而不是简单地重复标题
- 为了提高无障碍性,考虑添加
aria-label
属性来描述标签页的用途- <Tabs.TabPane title="第一名top1">第一名top1</Tabs.TabPane> + <Tabs.TabPane + title="第一名top1" + aria-label="展示排名第一的商品信息" + > + {/* 这里添加实际的商品内容 */} + <div>商品详情内容...</div> + </Tabs.TabPane>src/packages/tabs/tabs.tsx (1)
189-191
: 建议优化性能!可以考虑使用
useCallback
优化tabChange
回调函数,避免不必要的重渲染。+ const memoizedTabChange = useCallback((item: TabsTitle) => { + onClick?.(item.value) + if (!item.disabled) { + setValue(item.value) + } + }, [onClick, setValue]) return ( ... - onClick={() => tabChange(item)} + onClick={() => memoizedTabChange(item)} ... )Also applies to: 201-205
src/packages/tabs/tabs.taro.tsx (1)
227-230
: 建议使用可选链操作符可以使用可选链操作符来简化代码:
- onClick && onClick(item.value) + onClick?.(item.value)🧰 Tools
🪛 Biome (1.9.4)
[error] 229-230: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
src/packages/tabpane/tabpane.taro.tsx
(2 hunks)src/packages/tabpane/tabpane.tsx
(1 hunks)src/packages/tabs/__test__/tabs.spec.tsx
(8 hunks)src/packages/tabs/demos/h5/demo1.tsx
(1 hunks)src/packages/tabs/demos/h5/demo10.tsx
(1 hunks)src/packages/tabs/demos/h5/demo11.tsx
(1 hunks)src/packages/tabs/demos/h5/demo12.tsx
(2 hunks)src/packages/tabs/demos/h5/demo13.tsx
(1 hunks)src/packages/tabs/demos/h5/demo14.tsx
(1 hunks)src/packages/tabs/demos/h5/demo16.tsx
(1 hunks)src/packages/tabs/demos/h5/demo2.tsx
(1 hunks)src/packages/tabs/demos/h5/demo20.tsx
(1 hunks)src/packages/tabs/demos/h5/demo21.tsx
(1 hunks)src/packages/tabs/demos/h5/demo22.tsx
(2 hunks)src/packages/tabs/demos/h5/demo3.tsx
(1 hunks)src/packages/tabs/demos/h5/demo4.tsx
(1 hunks)src/packages/tabs/demos/h5/demo5.tsx
(1 hunks)src/packages/tabs/demos/h5/demo6.tsx
(1 hunks)src/packages/tabs/demos/h5/demo7.tsx
(1 hunks)src/packages/tabs/demos/h5/demo8.tsx
(1 hunks)src/packages/tabs/demos/h5/demo9.tsx
(1 hunks)src/packages/tabs/demos/taro/demo1.tsx
(1 hunks)src/packages/tabs/demos/taro/demo10.tsx
(1 hunks)src/packages/tabs/demos/taro/demo11.tsx
(1 hunks)src/packages/tabs/demos/taro/demo12.tsx
(2 hunks)src/packages/tabs/demos/taro/demo13.tsx
(1 hunks)src/packages/tabs/demos/taro/demo14.tsx
(1 hunks)src/packages/tabs/demos/taro/demo16.tsx
(1 hunks)src/packages/tabs/demos/taro/demo2.tsx
(1 hunks)src/packages/tabs/demos/taro/demo20.tsx
(1 hunks)src/packages/tabs/demos/taro/demo21.tsx
(1 hunks)src/packages/tabs/demos/taro/demo22.tsx
(2 hunks)src/packages/tabs/demos/taro/demo3.tsx
(1 hunks)src/packages/tabs/demos/taro/demo4.tsx
(1 hunks)src/packages/tabs/demos/taro/demo5.tsx
(1 hunks)src/packages/tabs/demos/taro/demo6.tsx
(1 hunks)src/packages/tabs/demos/taro/demo7.tsx
(1 hunks)src/packages/tabs/demos/taro/demo8.tsx
(1 hunks)src/packages/tabs/demos/taro/demo9.tsx
(1 hunks)src/packages/tabs/tabs.scss
(10 hunks)src/packages/tabs/tabs.taro.tsx
(8 hunks)src/packages/tabs/tabs.tsx
(5 hunks)
✅ Files skipped from review due to trivial changes (31)
- src/packages/tabs/demos/h5/demo5.tsx
- src/packages/tabs/demos/h5/demo10.tsx
- src/packages/tabs/demos/h5/demo14.tsx
- src/packages/tabs/demos/h5/demo3.tsx
- src/packages/tabs/demos/h5/demo7.tsx
- src/packages/tabs/demos/h5/demo1.tsx
- src/packages/tabs/demos/h5/demo6.tsx
- src/packages/tabs/demos/taro/demo5.tsx
- src/packages/tabs/demos/taro/demo10.tsx
- src/packages/tabs/demos/taro/demo6.tsx
- src/packages/tabs/demos/taro/demo4.tsx
- src/packages/tabs/demos/h5/demo8.tsx
- src/packages/tabs/demos/h5/demo2.tsx
- src/packages/tabs/demos/taro/demo2.tsx
- src/packages/tabs/demos/h5/demo9.tsx
- src/packages/tabs/demos/taro/demo9.tsx
- src/packages/tabs/demos/h5/demo21.tsx
- src/packages/tabs/demos/taro/demo13.tsx
- src/packages/tabs/demos/taro/demo8.tsx
- src/packages/tabs/demos/taro/demo1.tsx
- src/packages/tabs/demos/taro/demo14.tsx
- src/packages/tabs/demos/h5/demo22.tsx
- src/packages/tabs/demos/taro/demo21.tsx
- src/packages/tabs/demos/taro/demo20.tsx
- src/packages/tabs/demos/h5/demo4.tsx
- src/packages/tabs/demos/taro/demo7.tsx
- src/packages/tabs/demos/taro/demo3.tsx
- src/packages/tabs/demos/h5/demo12.tsx
- src/packages/tabs/demos/taro/demo11.tsx
- src/packages/tabs/demos/taro/demo22.tsx
- src/packages/tabs/demos/h5/demo20.tsx
🧰 Additional context used
🪛 GitHub Actions: CI
src/packages/tabs/__test__/tabs.spec.tsx
[error] 29-29: Test failed: Expected element to have class 'nut-tabs-titles-scrollable' but received 'nut-tabs-titles nut-tabs-titles-smile'
🪛 Biome (1.9.4)
src/packages/tabs/tabs.taro.tsx
[error] 229-230: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (17)
src/packages/tabs/demos/taro/demo16.tsx (1)
14-14
: 参考 H5 版本的改进建议此处存在与 H5 版本相同的问题,请参考上述建议进行相应修改。
src/packages/tabs/demos/taro/demo12.tsx (1)
17-17
: 代码变更看起来没有问题!标签文本从 "Tab 1" 更改为 "Tab longitem" 的修改保持了一致性。
Also applies to: 31-31
src/packages/tabs/demos/h5/demo13.tsx (1)
16-25
: 文本更新保持一致性,没有问题!所有 "Tab 1" 文本都已统一更新为 "Tab longitem",保持了整体一致性。
src/packages/tabs/__test__/tabs.spec.tsx (1)
10-10
: 文本更新保持一致性,没有问题!测试用例中的标签文本已统一更新。
Also applies to: 19-20, 35-36, 47-47, 74-75, 90-90, 96-97, 123-124, 148-149, 157-158
src/packages/tabs/tabs.tsx (4)
45-45
: 类型安全性改进很好!
- 使用
Partial<TabsProps>
提高了类型灵活性- 参数类型从
any
改为HTMLDivElement
增强了类型安全性Also applies to: 66-66, 73-73, 76-76, 78-80
86-89
: 代码简化得更清晰了!使用箭头函数重构
animate
函数使代码更简洁。
177-179
: 逻辑优化更合理!通过先检查
disabled
状态再设置值,避免了不必要的状态更新。
231-239
: 代码结构优化很好!优化了子组件的条件渲染逻辑,提高了代码可读性。
src/packages/tabs/tabs.taro.tsx (4)
70-71
: 代码简化获得认可!Props 解构的简化提高了代码的可读性。
84-86
: 代码健壮性改进!通过使用
React.isValidElement
进行适当的子元素检查,提高了代码的可靠性。
249-251
: 样式处理优化!直接传递 tabStyle 属性使代码更加清晰和易于维护。
303-311
: 类型安全性提升!改进了子组件的处理方式,增强了类型检查并优化了属性传递。
src/packages/tabs/demos/h5/demo11.tsx (1)
14-15
: 示例更新合理!标签文本从 "Tab 1" 更改为 "Tab longitem" 使示例更加清晰,并与其他示例保持一致。
src/packages/tabs/tabs.scss (4)
8-18
: 布局样式优化!
- 新增水平布局类提升了组件的灵活性
- 改进了溢出处理,优化了滚动体验
Line range hint
103-138
: 状态样式改进!
- 优化了激活状态的视觉反馈
- 统一了禁用状态的样式处理
329-339
: RTL 支持完善!添加了全面的从右到左布局支持,确保了组件在不同书写方向下的正确显示。
320-327
: 内容区域样式优化!新增的内容包装器类改善了布局结构和溢出处理。
return children ? ( | ||
<div className={classes}>{!disabled && children}</div> | ||
) : null | ||
return children && <div className={classes}>{!disabled && children}</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.
🛠️ Refactor suggestion
建议优化条件渲染逻辑
当前实现存在以下问题:
- 条件渲染逻辑可能不够直观
- disabled 状态的处理方式可能导致不一致的渲染结果
建议重构为更清晰的实现:
- return children && <div className={classes}>{!disabled && children}</div>
+ if (!children) return null
+ return (
+ <div className={classes}>
+ {disabled ? (
+ <div className={`${classPrefix}__disabled`}>
+ {children}
+ </div>
+ ) : children}
+ </div>
+ )
Committable suggestion skipped: line range outside the PR's diff.
@@ -1,5 +1,6 @@ | |||
import React, { FunctionComponent } from 'react' | |||
import classNames from 'classnames' | |||
import { View } from '@tarojs/components' |
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.
🛠️ Refactor suggestion
建议统一组件引入方式并优化渲染逻辑
- 建议将组件引入语句与其他引入保持一致的顺序
- 渲染逻辑存在与 React 版本相同的问题
- import { View } from '@tarojs/components'
+ import type { View } from '@tarojs/components'
+ import { View } from '@tarojs/components'
// ... 其他代码 ...
- return children && <View className={classes}>{!disabled && children}</View>
+ if (!children) return null
+ return (
+ <View className={classes}>
+ {disabled ? (
+ <View className={`${classPrefix}__disabled`}>
+ {children}
+ </View>
+ ) : children}
+ </View>
+ )
Also applies to: 41-41
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
发布说明
新功能
样式调整
性能优化
兼容性改进