-
Notifications
You must be signed in to change notification settings - Fork 263
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(Button): RN & 鸿蒙适配 #2382
fix(Button): RN & 鸿蒙适配 #2382
Conversation
Warning Review failedThe pull request is closed. Walkthrough这些变化是关于按钮组件的样式和功能的改进,涵盖了从样式文件到示例演示代码的广泛调整。具体包括对按钮的颜色、边框、背景、字体等属性的修改,以及在不同平台和状态下的条件样式处理。这些改进旨在提高按钮组件的视觉效果和可用性。 Changes
Sequence Diagram(s)无需生成。 Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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
src/packages/button/button copy.scss
Outdated
} | ||
|
||
&-warning { | ||
color: $button-warning-border-color; | ||
background-origin: border-box; | ||
border-width: $button-border-width; | ||
border-style: solid; | ||
border-color: $button-warning-border-color; | ||
|
||
&-solid { | ||
color: $button-warning-color; | ||
background: $button-warning-background-color; | ||
border-color: transparent; | ||
} | ||
|
||
&-dashed { | ||
border-style: dashed; | ||
} | ||
|
||
&-none { | ||
border-color: transparent; | ||
} | ||
|
||
&-disabled { | ||
background: transparent; | ||
color: $button-warning-disabled; | ||
border-color: $button-warning-disabled; | ||
} | ||
|
||
&-solid-disabled { | ||
color: $button-warning-color; | ||
background: $button-warning-disabled; | ||
} | ||
|
||
&-none-disabled { | ||
border-color: transparent; | ||
} | ||
} | ||
|
||
&-block { | ||
display: block; | ||
width: 100%; | ||
} | ||
|
||
&-solid { | ||
color: $button-primary-color; | ||
background: $button-primary-background-color; | ||
background-origin: border-box; | ||
border: $button-border-width solid transparent; | ||
|
||
&.nut-button-disabled { | ||
color: $button-default-disabled-color; | ||
background: $button-default-disabled; | ||
} | ||
} | ||
|
||
&-outline, | ||
&-dashed { | ||
background: transparent; | ||
} | ||
|
||
&-none { | ||
background: transparent; | ||
border-color: transparent; | ||
background: transparent; | ||
} | ||
|
||
&-disabled { | ||
cursor: not-allowed; | ||
color: #ffffff; | ||
background: $button-default-disabled; | ||
border-color: $button-default-disabled; | ||
} | ||
|
||
&-loading { | ||
cursor: default; | ||
opacity: 0.9; | ||
} | ||
|
||
&-round { | ||
border-radius: $button-border-radius; | ||
|
||
&.nut-button-xlarge, | ||
&.nut-button-large { | ||
border-radius: $button-large-border-radius; | ||
} | ||
|
||
&.nut-button-small { | ||
border-radius: $button-small-border-radius; | ||
} | ||
|
||
&.nut-button-mini { | ||
border-radius: $button-mini-border-radius; | ||
} | ||
} | ||
|
||
&-square { | ||
border-radius: $button-square-border-radius; | ||
} | ||
} |
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.
文件内容与button.scss
相似,但存在重复。
这个文件似乎是button.scss
的副本,包含了相同的样式指令。建议检查是否有必要保留此文件,或者考虑合并以避免代码冗余。
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.
备份文件已删除。错误提交
className={` ${props.fill || disabled || loading ? `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}` : ''} ${icon || loading ? 'nut-button-text' : ''}${ | ||
rightIcon ? ' nut-button-text-right' : '' |
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.
条件渲染复杂类名的优化
这里根据 props.fill
, disabled
, loading
状态动态生成类名。建议在这种复杂逻辑中添加注释,说明每个条件的用途,以提高代码的可读性。
- className={` ${props.fill || disabled || loading ? `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}` : ''} ${icon || loading ? 'nut-button-text' : ''}${
- rightIcon ? ' nut-button-text-right' : ''
- }`}
+ // 添加注释说明各个条件
+ className={` ${props.fill || disabled || loading ? `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}` : ''} ${icon || loading ? 'nut-button-text' : ''}${rightIcon ? ' nut-button-text-right' : ''}`}
Committable suggestion was skipped due to low confidence.
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: 4
[`${prefixCls}-${shape}-${size}`]: shape && size, | ||
[`${prefixCls}-block`]: block, | ||
[`${prefixCls}-disabled`]: disabled || loading, | ||
[`${prefixCls}-${type}${props.fill ? `-${fill}` : ''}-disabled`]: | ||
disabled || loading, |
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.
优化类名动态生成逻辑
此代码段负责根据不同的属性动态生成类名,但存在一些冗余和复杂的逻辑。可以通过简化条件判断来优化这部分代码,提高其可读性和可维护性。
- [`${prefixCls}-${type}${props.fill ? `-${fill}` : ''}-disabled`]:
+ [`${prefixCls}-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`]: true,
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.
[`${prefixCls}-${shape}-${size}`]: shape && size, | |
[`${prefixCls}-block`]: block, | |
[`${prefixCls}-disabled`]: disabled || loading, | |
[`${prefixCls}-${type}${props.fill ? `-${fill}` : ''}-disabled`]: | |
disabled || loading, | |
[`${prefixCls}-${shape}-${size}`]: shape && size, | |
[`${prefixCls}-block`]: block, | |
[`${prefixCls}-disabled`]: disabled || loading, | |
[`${prefixCls}-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`]: true, |
className={`nut-button-children nut-button-${size}-children nut-button-${type}-children ${!(props.fill || disabled || loading) ? '' : `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`} ${icon || loading ? `nut-button-text` : ''}${ | ||
rightIcon ? ` nut-button-text-right` : '' | ||
}`} | ||
style={harmonyAndRn() ? getContStyle() : ''} |
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.
优化子组件类名和样式设置
此代码段负责为子组件设置类名和样式。建议优化这一逻辑,以减少复杂性并提高代码的清晰度。
+ // 根据条件动态设置类名,减少代码复杂性
- className={`nut-button-children nut-button-${size}-children nut-button-${type}-children ${!(props.fill || disabled || loading) ? '' : `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`} ${icon || loading ? `nut-button-text` : ''}${
+ className={classNames('nut-button-children', `nut-button-${size}-children`, `nut-button-${type}-children`, {
+ [`nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`]: props.fill || disabled || loading,
+ 'nut-button-text': icon || loading,
+ 'nut-button-text-right': rightIcon,
+ })}
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.
className={`nut-button-children nut-button-${size}-children nut-button-${type}-children ${!(props.fill || disabled || loading) ? '' : `nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`} ${icon || loading ? `nut-button-text` : ''}${ | |
rightIcon ? ` nut-button-text-right` : '' | |
}`} | |
style={harmonyAndRn() ? getContStyle() : ''} | |
className={classNames('nut-button-children', `nut-button-${size}-children`, `nut-button-${type}-children`, { | |
[`nut-button-${type}${props.fill ? `-${fill}` : ''}${disabled || loading ? '-disabled' : ''}`]: props.fill || disabled || loading, | |
'nut-button-text': icon || loading, | |
'nut-button-text-right': rightIcon, | |
})} | |
style={harmonyAndRn() ? getContStyle() : ''} |
if (harmonyAndRn()) { | ||
style.backgroundColor = color | ||
} |
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.
调整背景颜色设置以适应不同平台
此代码段针对鸿蒙和 React Native 平台进行了特别的背景颜色设置。建议在此基础上添加注释,说明为什么需要这样做,以提高代码的可读性和可维护性。
+ // 当运行平台为鸿蒙或React Native时,设置特定的背景颜色
if (harmonyAndRn()) {
style.backgroundColor = color
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.
if (harmonyAndRn()) { | |
style.backgroundColor = color | |
} | |
// 当运行平台为鸿蒙或React Native时,设置特定的背景颜色 | |
if (harmonyAndRn()) { | |
style.backgroundColor = color |
src/packages/button/button.taro.tsx
Outdated
const getContStyle = useCallback(() => { | ||
const style: CSSProperties = {} | ||
if (props.color) { | ||
if (props.fill === 'outline' || props.fill === 'dashed') { | ||
style.color = color | ||
if (!color?.includes('gradient')) { | ||
style.borderColor = color | ||
} | ||
} else { | ||
style.color = '#fff' | ||
style.background = 'transparent' | ||
style.borderColor = 'transparent' | ||
} | ||
} | ||
return style | ||
}, [color]) |
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.
优化 getContStyle
函数的实现
这个函数用于根据按钮的不同状态和属性动态生成样式。代码实现功能正确,但可以进一步优化以提高性能,例如通过减少重复代码和利用更简洁的条件表达式。
+ // 使用更简洁的条件判断来设置样式
if (props.color) {
if (props.fill === 'outline' || props.fill === 'dashed') {
style.color = color
if (!color?.includes('gradient')) {
style.borderColor = color
}
} else {
style.color = '#fff'
style.background = 'transparent'
style.borderColor = 'transparent'
}
}
return style
Committable suggestion was skipped due to low confidence.
|
||
const Demo4 = () => { | ||
const marginStyle = { margin: 8 } | ||
const marginStyle = { | ||
marginRight: 8, |
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.
都用pxTransform转一下,现在鸿蒙间距有点小
marginLeft: 8, | ||
marginBottom: 8, | ||
} | ||
: { |
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.
{
marginRight: 8,
marginTop: 8,
marginLeft: 8,
marginBottom: 8,
}可以考虑提取一下
@@ -22,17 +27,18 @@ | |||
-webkit-tap-highlight-color: rgba(0, 0, 0, 0); | |||
color: #1a1a1a; | |||
background: transparent; |
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.
harmony背景色透明覆盖不了button默认的灰色,$button-default-background-color是否需要改成白色
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
样式
功能
组件
示例