-
Notifications
You must be signed in to change notification settings - Fork 280
feat(shortpassword): harmony #3017
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
feat(shortpassword): harmony #3017
Conversation
Walkthrough此次变更主要涉及 ShortPassword 组件相关的配置、样式和接口更新。配置文件中启用了 ShortPassword 组件的 "dd" 属性;样式文件中新增了弹窗样式以及调整了标题和描述的居中对齐;组件实现中添加了 Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3017 +/- ##
=============================================
+ Coverage 86.05% 86.11% +0.06%
=============================================
Files 281 283 +2
Lines 18255 18399 +144
Branches 2749 2768 +19
=============================================
+ Hits 15709 15845 +136
- Misses 2541 2549 +8
Partials 5 5 ☔ View full report in Codecov by Sentry. |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/config.json
(2 hunks)src/packages/overlay/overlay.taro.tsx
(1 hunks)src/packages/shortpassword/shortpassword.scss
(1 hunks)src/packages/shortpassword/shortpassword.taro.tsx
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/packages/overlay/overlay.taro.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (8)
src/config.json (2)
173-173
: 空行删除使代码更整洁。移除此空行使布局组件部分的代码结构更加紧凑、整洁。
780-780
: 启用 ShortPassword 组件。通过将
"dd"
属性从false
改为true
,启用了短密码组件,使其可在项目中使用。此变更与shortpassword
目录下的组件更新保持一致。src/packages/shortpassword/shortpassword.scss (2)
11-12
: 标题居中对齐优化。添加 flex 布局使标题能够居中对齐,提升了界面美观度。
19-20
: 描述文本居中对齐优化。添加 flex 布局使描述文本能够居中对齐,与标题保持一致的样式,提升了组件视觉统一性。
src/packages/shortpassword/shortpassword.taro.tsx (4)
40-40
: 添加默认 zIndex 值。为 ShortPassword 组件添加默认的 zIndex 值,使其在层叠上下文中有确定的位置,有助于解决与其他弹出层组件的堆叠问题。
66-67
: 增加 zIndex 和 round 属性。在组件属性中解构出 zIndex 和 round 属性,增强了组件的可配置性,使其更加灵活。
128-128
: 从内联样式迁移到基于类的样式。将 Popup 组件的样式从内联方式改为使用类名,遵循了更好的样式管理实践,使样式更加可维护和统一。
133-134
: 为 Popup 组件传递 zIndex 和 round 属性。将 zIndex 和 round 属性传递给 Popup 组件,确保正确的堆叠顺序和圆角效果,提升了组件的视觉表现和用户体验。
&-popup { | ||
padding: 32px 24px 28px 24px; | ||
borderradius: 12px; | ||
textalign: center; | ||
} |
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.
CSS 属性命名格式错误。
borderradius
和 textalign
属性命名格式不正确,应使用连字符格式。
请修改如下:
&-popup {
padding: 32px 24px 28px 24px;
- borderradius: 12px;
- textalign: center;
+ border-radius: 12px;
+ text-align: center;
}
📝 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.
&-popup { | |
padding: 32px 24px 28px 24px; | |
borderradius: 12px; | |
textalign: center; | |
} | |
&-popup { | |
padding: 32px 24px 28px 24px; | |
border-radius: 12px; | |
text-align: center; | |
} |
# Conflicts: # src/config.json # src/packages/overlay/overlay.taro.tsx
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
🧹 Nitpick comments (2)
src/packages/shortpassword/shortpassword.taro.tsx (2)
46-49
: 建议更新组件接口类型考虑到您添加了新的
round
属性,应更新InternalShortPassword
的类型定义,以确保新属性被正确地包含在类型中。export const InternalShortPassword: ForwardRefRenderFunction< unknown, - Partial<ShortPasswordProps> + Partial<ShortPasswordProps> > = (props, ref) => {您无需修改此处的类型定义,只需在
ShortPasswordProps
接口中添加round
属性,因为这里使用了该接口。
37-45
: 考虑为 round 属性添加默认值为了保持一致性,建议在
defaultProps
中为新添加的round
属性设置默认值。const defaultProps = { ...ComponentDefaults, value: '', visible: false, plain: false, hideFooter: true, length: 6, // 1~6 autoFocus: false, + round: false, } as ShortPasswordProps
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/shortpassword/shortpassword.taro.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
src/packages/shortpassword/shortpassword.taro.tsx (1)
126-126
: 良好的样式重构将内联样式改为基于类名的样式是一个很好的做法,这样可以提高代码的可维护性和样式的复用性。
visible={visible} | ||
closeable | ||
onOverlayClick={onClose} | ||
onCloseIconClick={onClose} | ||
round |
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
应当使用 props 中的 round 值而非硬编码
目前您在 Popup 组件上硬编码了 round
属性为 true,但更好的做法是使用从 props 中解构出来的 round
变量,这样可以让组件的使用者控制该属性。
<Popup
className={`${classPrefix}-popup`}
visible={visible}
closeable
onOverlayClick={onClose}
onCloseIconClick={onClose}
- round
+ round={round}
{...rest}
>
📝 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.
round | |
<Popup | |
className={`${classPrefix}-popup`} | |
visible={visible} | |
closeable | |
onOverlayClick={onClose} | |
onCloseIconClick={onClose} | |
round={round} | |
{...rest} | |
> |
@@ -62,6 +62,7 @@ | |||
className, | |||
closeable, | |||
autoFocus, | |||
round, |
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.
需要更新 ShortPasswordProps 接口
您在组件的析构中添加了 round
参数,但没有在 ShortPasswordProps
接口中声明此属性。为确保类型安全和文档完整性,应当更新接口定义。
建议在 ShortPasswordProps 接口中添加 round 属性定义:
export interface ShortPasswordProps extends PopupProps {
value: string
visible: boolean
plain: boolean
title: ReactNode
description: ReactNode
tips: ReactNode
hideFooter: boolean
length: number
error: ReactNode
autoFocus: boolean
+ round?: boolean
onFocus: () => void
onChange: (value: string) => void
onConfirm: (value: string) => void
onCancel: () => void
onClose: () => void
onTips: () => void
onComplete: (value: string) => void
}
📝 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.
round, | |
export interface ShortPasswordProps extends PopupProps { | |
value: string | |
visible: boolean | |
plain: boolean | |
title: ReactNode | |
description: ReactNode | |
tips: ReactNode | |
hideFooter: boolean | |
length: number | |
error: ReactNode | |
autoFocus: boolean | |
round?: boolean | |
onFocus: () => void | |
onChange: (value: string) => void | |
onConfirm: (value: string) => void | |
onCancel: () => void | |
onClose: () => void | |
onTips: () => void | |
onComplete: (value: string) => void | |
} |
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.
web h5 版本也要关注一下~~ 同样的处理
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: 1
🔭 Outside diff range comments (1)
src/packages/shortpassword/shortpassword.tsx (1)
46-48
: 🛠️ Refactor suggestion类型定义需要更新以支持 round 属性。
由于增加了 round 属性,函数组件的类型定义也应该更新。
建议修改为:
export const InternalShortPassword: ForwardRefRenderFunction< unknown, - Partial<ShortPasswordProps> + Partial<ShortPasswordProps & { round?: boolean }> > = (props, ref) => {或者更好的方法是直接在 ShortPasswordProps 接口中添加 round 属性的定义。
🧹 Nitpick comments (2)
src/packages/shortpassword/shortpassword.tsx (2)
125-131
: 考虑使 round 属性可配置而非硬编码。目前 round 属性是硬编码的,而不是基于传入的 props。这可能限制了组件的灵活性。
建议修改为:
<Popup className={`${classPrefix}-popup`} visible={visible} closeable onOverlayClick={handleClose} onCloseIconClick={handleClose} - round + round={round} {...rest} >这样,round 属性将成为可选的配置项,提高组件的灵活性和可定制性。
143-158
: 考虑优化密码框渲染逻辑,提高可读性。当前密码框的渲染逻辑嵌套较深,可以考虑提取为单独的函数或组件,以提高代码的可读性和可维护性。
建议考虑将密码输入框的渲染逻辑抽取为一个独立的函数:
+ const renderPasswordItem = (index: number) => { + return ( + <div className={`${classPrefix}-input-fake-li`} key={index}> + {inputValue.length > index && ( + <> + {plain ? ( + inputValue[index] + ) : ( + <div className={`${classPrefix}-input-fake-li-icon`} /> + )} + </> + )} + </div> + ) + } ... <div className={`${classPrefix}-input-fake`}> - {[...new Array(comLen).keys()].map((item, index) => { - return ( - <div className={`${classPrefix}-input-fake-li`} key={index}> - {inputValue.length > index && ( - <> - {plain ? ( - inputValue[index] - ) : ( - <div className={`${classPrefix}-input-fake-li-icon`} /> - )} - </> - )} - </div> - ) - })} + {[...new Array(comLen).keys()].map(renderPasswordItem)} </div>🧰 Tools
🪛 Biome (1.9.4)
[error] 147-153: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/shortpassword/shortpassword.scss
(1 hunks)src/packages/shortpassword/shortpassword.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/shortpassword/shortpassword.scss
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
src/packages/shortpassword/shortpassword.tsx (2)
125-125
: 优化:使用类名替代内联样式是良好的实践。将样式从内联方式改为基于类名的方式有利于代码的维护性和可读性,符合关注点分离原则。
130-130
:✅ Verification successful
验证 round 属性在 Popup 组件中的有效性。
这里为 Popup 组件添加了 round 属性,但需要确认 Popup 组件是否支持此属性。
🏁 Script executed:
#!/bin/bash # 检查 Popup 组件是否支持 round 属性 grep -r "round" --include="*.tsx" --include="*.ts" src/packages/popup/Length of output: 1291
验证 Popup 组件的 round 属性已有效实现
经过检查,Popup 组件在以下几个方面均正确支持
round
属性:
- 在
src/packages/popup/types.ts
中,已经定义了round: boolean
。- 在
src/packages/popup/popup.tsx
和对应 Taro 文件中,根据round
属性动态添加了类名(例如:${classPrefix}-round
),同时也结合了position==='bottom'
的逻辑。- 单元测试(见
src/packages/popup/__tests__/popup.spec.tsx
)验证了当启用round
属性时,会正确添加.nut-popup-round
样式。因此,当前实现完全满足对 Popup 组件的
round
属性的需求,无需进一步修改。
@@ -61,6 +61,7 @@ export const InternalShortPassword: ForwardRefRenderFunction< | |||
className, | |||
closeable, | |||
autoFocus, | |||
round, |
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.
新增的 round 属性缺少在 ShortPasswordProps 接口中的声明。
目前 round 属性仅在组件内部进行了解构,但未在 ShortPasswordProps 接口(第16-34行)中声明。这可能导致 TypeScript 类型检查警告或错误。
建议在 ShortPasswordProps 接口中添加 round 属性定义:
export interface ShortPasswordProps extends PopupProps {
value: string
visible: boolean
plain: boolean
title: ReactNode
description: ReactNode
tips: ReactNode
hideFooter: boolean
length: number
error: ReactNode
autoFocus: boolean
+ round?: boolean
onFocus: () => void
onChange: (value: string) => void
onConfirm: (value: string) => void
onCancel: () => void
onClose: () => void
onTips: () => void
onComplete: (value: string) => void
}
📝 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.
round, | |
export interface ShortPasswordProps extends PopupProps { | |
value: string | |
visible: boolean | |
plain: boolean | |
title: ReactNode | |
description: ReactNode | |
tips: ReactNode | |
hideFooter: boolean | |
length: number | |
error: ReactNode | |
autoFocus: boolean | |
round?: boolean | |
onFocus: () => void | |
onChange: (value: string) => void | |
onConfirm: (value: string) => void | |
onCancel: () => void | |
onClose: () => void | |
onTips: () => void | |
onComplete: (value: string) => void | |
} |
Summary by CodeRabbit
新功能
样式