-
Notifications
You must be signed in to change notification settings - Fork 269
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(resultpage): 适配 V14 #2741
fix(resultpage): 适配 V14 #2741
Changes from 2 commits
519a74d
873a635
7fd671f
9ffa71f
dea7964
e161e9d
1b15d7e
b8fd5d7
9643012
c8bef3a
eae8c4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,10 @@ const Demo1 = () => { | |
status="error" | ||
actions={[ | ||
{ | ||
text: '次要按钮', | ||
text: '操作按钮', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 按钮文案需要统一规范 发现
建议将所有按钮文案统一修改为 "操作按钮",以保持整体设计规范的一致性。 🔗 Analysis chain按钮文案修改符合规范! 按钮文案从"次要按钮"和"主要按钮"统一更新为"操作按钮"的修改符合V14版本的设计规范。 运行以下脚本验证其他demo文件中的按钮文案是否已统一更新: Also applies to: 16-16 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 1190 |
||
}, | ||
{ | ||
text: '主要按钮', | ||
text: '操作按钮', | ||
type: 'primary', | ||
}, | ||
]} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,25 +6,25 @@ const Demo1 = () => { | |||||
<> | ||||||
<Cell> | ||||||
<ResultPage | ||||||
title="信息补充" | ||||||
title="信息反馈" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 文案需要统一调整 根据代码库检查结果:
建议:
🔗 Analysis chain标题和按钮文案的一致性更新 文案更新符合设计规范,建议确认以下几点:
Also applies to: 14-14 Let me verify the title consistency as well before making the final response. 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 1209 Script:
Length of output: 720 |
||||||
description="内容描述内容可折行,建议最多不超过两行建议最多不超过两行" | ||||||
status="info" | ||||||
actions={[ | ||||||
{ | ||||||
text: '主要按钮', | ||||||
text: '主要操作', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 按钮文本不一致 根据其他 demo 文件的修改,此处的按钮文本应该使用"操作按钮"而不是"主要操作",以保持整体一致性。 建议修改如下:
📝 Committable suggestion
Suggested change
|
||||||
type: 'primary', | ||||||
}, | ||||||
]} | ||||||
/> | ||||||
</Cell> | ||||||
<Cell> | ||||||
<ResultPage | ||||||
title="信息补充" | ||||||
title="信息反馈" | ||||||
description="内容描述内容可折行,建议最多不超过两行建议最多不超过两行" | ||||||
status="info" | ||||||
actions={[ | ||||||
{ | ||||||
text: '次要按钮', | ||||||
text: '操作按钮', | ||||||
}, | ||||||
]} | ||||||
/> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,10 @@ const Demo1 = () => { | |
status="warning" | ||
actions={[ | ||
{ | ||
text: '次要按钮', | ||
text: '操作按钮', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 按钮文案需要进一步统一 在检查结果中发现,大部分demo文件已经统一使用了"操作按钮"作为按钮文案,但是在
🔗 Analysis chain按钮文案统一更新已完成 按钮文案的统一更新符合V14版本的规范要求,使用"操作按钮"替换原有的"次要按钮"和"主要按钮",提升了用户界面的一致性。 运行以下脚本验证其他demo文件中的按钮文案是否已统一更新: Also applies to: 15-15 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 1201 |
||
}, | ||
{ | ||
text: '主要按钮', | ||
text: '操作按钮', | ||
type: 'primary', | ||
}, | ||
]} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,21 +6,23 @@ | |
margin: 0 auto; | ||
} | ||
.nut-resultpage-icon { | ||
height: 48px; | ||
width: 48px; | ||
margin-bottom: 16px; | ||
background-size: 100%; | ||
display: inline-flex; | ||
margin-bottom: 12px; | ||
} | ||
.nut-resultpage-icon .nut-icon { | ||
height: 36px; | ||
width: 36px; | ||
} | ||
.nut-resultpage-title { | ||
width: 247px; | ||
margin-bottom: 9px; | ||
font-size: 20px; | ||
width: 240px; | ||
margin-bottom: 12px; | ||
font-size: 18px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 是不是应该使用 sass 变量,包括 margin-bottom There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个是编译后的css文件 |
||
color: #1a1a1a; | ||
font-weight: 600; | ||
text-align: center; | ||
} | ||
.nut-resultpage-description { | ||
width: 247px; | ||
width: 240px; | ||
line-height: 20px; | ||
font-size: 14px; | ||
color: #505259; | ||
|
@@ -33,12 +35,9 @@ | |
.nut-resultpage-actions { | ||
display: flex; | ||
flex-direction: row; | ||
margin-top: 21px; | ||
margin-top: 16px; | ||
} | ||
.nut-resultpage-action { | ||
margin-left: 6px; | ||
margin-right: 6px; | ||
} | ||
.nut-resultpage-action-button { | ||
white-space: nowrap; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,10 +5,12 @@ | |
align-items: center; | ||
margin: 0 auto; | ||
&-icon { | ||
height: $resultpage-icon-size; | ||
width: $resultpage-icon-size; | ||
display: inline-flex; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 有一种情况,如果用户使用的不是 nut icon库,会怎么样呢~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 那就传入自定义icon |
||
margin-bottom: $resultpage-icon-margin-bottom; | ||
background-size: 100%; | ||
.nut-icon { | ||
height: $resultpage-icon-size; | ||
width: $resultpage-icon-size; | ||
} | ||
Comment on lines
+8
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Flex布局实现合理,但建议补充响应式支持 根据代码分析,当前实现存在以下情况:
建议:
🔗 Analysis chain请验证响应式布局表现 新的 flex 布局可能在不同屏幕尺寸下有不同表现,建议验证以下场景:
🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 128 Script:
Length of output: 313 Script:
Length of output: 4405 |
||
} | ||
&-title { | ||
width: $resultpage-width; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import React, { FunctionComponent, ReactNode, useState } from 'react' | ||
import React, { FunctionComponent, ReactNode } from 'react' | ||
import classNames from 'classnames' | ||
import { View, Text, Image } from '@tarojs/components' | ||
import { Tips, Warning, Failure, Ask, Success } from '@nutui/icons-react-taro' | ||
import { View, Text } from '@tarojs/components' | ||
import { BasicComponent, ComponentDefaults } from '@/utils/typings' | ||
import { | ||
Button, | ||
|
@@ -18,6 +19,7 @@ export type ResultPageAction = { | |
disabled?: boolean | ||
onClick?: () => () => void | ||
} | ||
|
||
export interface ResultPageProps extends BasicComponent { | ||
title: ReactNode | ||
description: ReactNode | ||
|
@@ -54,24 +56,17 @@ export const ResultPage: FunctionComponent< | |
const cls = classNames(classPrefix, className) | ||
|
||
const defaultStatus: ResultPageStatusOptions = { | ||
success: | ||
'https://img11.360buyimg.com/imagetools/jfs/t1/233690/33/17768/1251/66543101F2589003b/f5dcaea8e29c23aa.png', | ||
error: | ||
'https://img14.360buyimg.com/imagetools/jfs/t1/224715/38/17932/1426/66543101F1dbc50e8/eaffdb926b00584b.png', | ||
warning: | ||
'https://img13.360buyimg.com/imagetools/jfs/t1/200545/24/43507/1219/66543101Fe58cfbe4/c29054555f3bcb4b.png', | ||
info: 'https://img13.360buyimg.com/imagetools/jfs/t1/199421/5/44230/1320/66543100F23a9466e/0e9eadd5cebf07fb.png', | ||
waiting: | ||
'https://img11.360buyimg.com/imagetools/jfs/t1/226266/21/17859/1428/66543101F2dc4c3f3/44e2ae2b51c6e0ed.png', | ||
success: <Success color="#00D900" />, | ||
error: <Failure color="#FF0F23" />, | ||
warning: <Warning color="#FFBF00" />, | ||
info: <Tips color="#0073FF" />, | ||
waiting: <Ask color="#0073FF" />, | ||
} | ||
const [iconSrc] = useState<any>( | ||
`${typeof icon === 'string' && icon ? icon : defaultStatus[status]}` | ||
) | ||
return ( | ||
<View className={cls} style={style}> | ||
{typeof icon === 'string' ? ( | ||
<Image src={iconSrc} className={`${classPrefix}-icon`} /> | ||
) : null} | ||
<View className={`${classPrefix}-icon`}> | ||
{icon || defaultStatus[status]} | ||
</View> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. icon 这个可以参考一下 pagination 的处理~ 我觉得那个方法还不错 |
||
{typeof title === 'string' && title ? ( | ||
<View className={`${classPrefix}-title`}>{title}</View> | ||
) : ( | ||
|
@@ -90,7 +85,7 @@ export const ResultPage: FunctionComponent< | |
const { text, ...rest } = action | ||
return ( | ||
<View className={`${classPrefix}-action`} key={index}> | ||
<Button {...rest} size="small"> | ||
<Button {...rest} size="large"> | ||
{action?.text} | ||
</Button> | ||
</View> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import React, { FunctionComponent, ReactNode, useState } from 'react' | ||
import React, { FunctionComponent, ReactNode } from 'react' | ||
import classNames from 'classnames' | ||
import { Tips, Warning, Failure, Ask, Success } from '@nutui/icons-react' | ||
import { BasicComponent, ComponentDefaults } from '@/utils/typings' | ||
import { Button, ButtonFill, ButtonSize, ButtonType } from '../button/button' | ||
import { ResultPageStatus, ResultPageStatusOptions } from './types' | ||
|
@@ -49,27 +50,17 @@ | |
const cls = classNames(classPrefix, className) | ||
|
||
const defaultStatus: ResultPageStatusOptions = { | ||
success: | ||
'https://img11.360buyimg.com/imagetools/jfs/t1/233690/33/17768/1251/66543101F2589003b/f5dcaea8e29c23aa.png', | ||
error: | ||
'https://img14.360buyimg.com/imagetools/jfs/t1/224715/38/17932/1426/66543101F1dbc50e8/eaffdb926b00584b.png', | ||
warning: | ||
'https://img13.360buyimg.com/imagetools/jfs/t1/200545/24/43507/1219/66543101Fe58cfbe4/c29054555f3bcb4b.png', | ||
info: 'https://img13.360buyimg.com/imagetools/jfs/t1/199421/5/44230/1320/66543100F23a9466e/0e9eadd5cebf07fb.png', | ||
waiting: | ||
'https://img11.360buyimg.com/imagetools/jfs/t1/226266/21/17859/1428/66543101F2dc4c3f3/44e2ae2b51c6e0ed.png', | ||
success: <Success color="#00D900" />, | ||
error: <Failure color="#FF0F23" />, | ||
warning: <Warning color="#FFBF00" />, | ||
info: <Tips color="#0073FF" />, | ||
waiting: <Ask color="#0073FF" />, | ||
Comment on lines
+53
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 建议将图标颜色提取为主题变量 当前的颜色值是硬编码的,建议将这些颜色值提取到主题配置中,以便统一管理和维护:
|
||
} | ||
const [imgStyle] = useState<any>({ | ||
backgroundImage: `url(${typeof icon === 'string' && icon ? icon : defaultStatus[status]})`, | ||
backgroundSize: '100% 100%', | ||
}) | ||
return ( | ||
<div className={cls} style={style}> | ||
{typeof icon === 'string' ? ( | ||
<div className={`${classPrefix}-icon`} style={imgStyle} /> | ||
) : ( | ||
icon | ||
)} | ||
<div className={`${classPrefix}-icon`}> | ||
{icon || defaultStatus[status]} | ||
</div> | ||
{typeof title === 'string' && title ? ( | ||
<div className={`${classPrefix}-title`}>{title}</div> | ||
) : ( | ||
|
@@ -86,7 +77,9 @@ | |
const { text, ...rest } = action | ||
return ( | ||
<div className={`${classPrefix}-action`} key={index}> | ||
<Button {...rest}>{action?.text}</Button> | ||
<Button {...rest} size="large"> | ||
{action?.text} | ||
</Button> | ||
Comment on lines
+80
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification 需要补充按钮尺寸的测试用例 根据代码分析,ResultPage 组件的测试文件中缺少对按钮尺寸属性的测试覆盖。建议在
具体测试点:
🔗 Analysis chain按钮尺寸标准化需要补充测试 统一使用 "large" 尺寸的更改是合理的,但缺少测试覆盖。建议添加相关测试用例。 需要验证按钮在不同场景下的表现: 是否需要我帮助编写测试用例? Let me check the test coverage for this specific component and its button size usage. Let me check the content of the existing test file to see what's already covered. 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 845 Script:
Length of output: 567 Script:
Length of output: 1531 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 81-81: src/packages/resultpage/resultpage.tsx#L81 |
||
</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.
次要和主要,主要是搭配着 按钮的语义来写的~可以保留
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.
都统一改成 主要操作、次要操作、信息反馈 这些文案吧