OK-34496: Confirm passphrase use enter key #6397
Conversation
originalix
requested review from
ezailWang,
loatheb,
sidmorizon and
huhuanming
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
概述代码变更总览工作流程此次代码变更主要涉及四个文件，重点是改进令牌管理、硬件交互和搜索功能的异步处理。主要变更包括在 变更详情
序列图对于 sequenceDiagram
participant Client
participant ServiceToken
participant CustomTokens
Client->>ServiceToken: mergeTokenMetadataWithCustomData()
ServiceToken->>CustomTokens: 获取自定义令牌
CustomTokens-->>ServiceToken: 返回自定义令牌
ServiceToken->>ServiceToken: 合并令牌元数据
ServiceToken-->>Client: 返回合并后的令牌
这个序列图展示了新的异步令牌元数据合并方法的工作原理，突出了客户端、服务和自定义令牌之间的交互。 Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/kit-bg/src/services/ServiceToken.ts(5 hunks)packages/kit/src/components/Hardware/Hardware.tsx(3 hunks)packages/kit/src/views/AssetList/hooks/useTokenManagement.ts(1 hunks)packages/kit/src/views/Discovery/pages/SearchModal/index.tsx(1 hunks)
🔇 Additional comments (2)
packages/kit/src/components/Hardware/Hardware.tsx (2)
379-388: 事件卸载完善
组件卸载时能正确移除监听器，做法合理，不需额外改动。
483-483: 逻辑统一
将点击确认的逻辑统一到 handleSubmit，有助于维护与调试。
| const allTokens = await Promise.all( | ||
| [...tokenList.tokens, ...customTokens].map((token) => | ||
| backgroundApiProxy.serviceToken.mergeTokenMetadataWithCustomData({ | ||
| token, | ||
| customTokens, | ||
| networkId, | ||
| }), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
添加错误处理
这里使用了 Promise.all()，如果其中任意一个 Promise 被拒绝，会阻塞整个流程。建议在外层添加 try/catch 或分块执行，防止单点错误导致全部失败。
- const allTokens = await Promise.all([...]);
+ let allTokens = [];
+ try {
+ allTokens = await Promise.all([...]);
+ } catch (error) {
+ // 在这里记录并安全处理错误
+ }Committable suggestion skipped: line range outside the PR's diff.
| await backgroundApiProxy.serviceDiscovery.buildWebsiteIconUrl( | ||
| 'https://google.com', | ||
| ); | ||
| const logo = 'https://uni.onekey-asset.com/static/logo/google.png'; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
避免硬编码 URL
建议将该 logo 地址配置化或集中管理，后续维护更灵活。
| @@ -21,6 +21,7 @@ import { | |||
| useMedia, | |||
| } from '@onekeyhq/components'; | |||
| import { ETranslations } from '@onekeyhq/shared/src/locale'; | |||
| import platformEnv from '@onekeyhq/shared/src/platformEnv'; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
确认导入用途
如果只在浏览器环境使用，可考虑按需加载，避免在其它平台上额外增大包体。
| const handleSubmit = form.handleSubmit(async () => { | ||
| const values = form.getValues(); | ||
| if ( | ||
| !isSingleInput && | ||
| (values.passphrase || '') !== (values.confirmPassphrase || '') | ||
| ) { | ||
| Toast.error({ | ||
| title: intl.formatMessage({ | ||
| id: ETranslations.feedback_passphrase_not_matched, | ||
| }), | ||
| }); | ||
| return; | ||
| } | ||
| const passphrase = values.passphrase || ''; | ||
| onConfirm({ passphrase, save: true }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
检查空字符串
当 passphrase 为空时，最好给用户额外提示，以免误操作。
| const handleKeyPress = useCallback( | ||
| async (event: KeyboardEvent) => { | ||
| if (event.key === 'Enter') { | ||
| void handleSubmit(); | ||
| } | ||
| }, | ||
| [handleSubmit], | ||
| ); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
留意回车事件
如果用户想换行而不小心触发回车提交，可能带来操作困扰。可区分或提醒。
| @@ -207,7 +207,7 @@ | |||
|
|
|||
| resp.data.data.riskTokens.data = resp.data.data.riskTokens.data.map( | |||
| (token) => ({ | |||
| ...this.mergeTokenMetadataWithCustomData({ | |||
| ...this.mergeTokenMetadataWithCustomDataSync({ | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
重复调用
这一段再次调用相同逻辑，封装可提高代码可维护性。
| @@ -220,7 +220,7 @@ | |||
|
|
|||
| resp.data.data.smallBalanceTokens.data = | |||
| resp.data.data.smallBalanceTokens.data.map((token) => ({ | |||
| ...this.mergeTokenMetadataWithCustomData({ | |||
| ...this.mergeTokenMetadataWithCustomDataSync({ | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
减少重复
这里也重复了相似逻辑，建议统一抽离成公共方法。
| return Promise.resolve(this.mergeTokenMetadataWithCustomDataSync(params)); | ||
| } | ||
|
|
||
| private mergeTokenMetadataWithCustomDataSync<T extends IToken>({ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
可扩展性建议
仅校验 token.symbol 和 token.name，如果有更多字段要合并或覆盖，建议在此一并处理。
| @@ -521,7 +530,7 @@ | |||
| networkId, | |||
| }); | |||
|
|
|||
| tokenInfo = this.mergeTokenMetadataWithCustomData({ | |||
| tokenInfo = this.mergeTokenMetadataWithCustomDataSync({ | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
复用逻辑
此处与 fetchAccountTokens 里的逻辑类似，建议共用合并方法，减少重复。
| @backgroundMethod() | ||
| async mergeTokenMetadataWithCustomData<T extends IToken>(params: { | ||
| token: T; | ||
| customTokens: IAccountToken[]; | ||
| networkId: string; | ||
| }): Promise<T> { | ||
| return Promise.resolve(this.mergeTokenMetadataWithCustomDataSync(params)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
直接使用 async/await
无需手动调用 Promise.resolve()，可直接返回结果，简化代码结构。
async mergeTokenMetadataWithCustomData<T extends IToken>(params: {
token: T;
customTokens: IAccountToken[];
networkId: string;
}): Promise<T> {
- return Promise.resolve(this.mergeTokenMetadataWithCustomDataSync(params));
+ return this.mergeTokenMetadataWithCustomDataSync(params);
}📝 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.
| @backgroundMethod() | |
| async mergeTokenMetadataWithCustomData<T extends IToken>(params: { | |
| token: T; | |
| customTokens: IAccountToken[]; | |
| networkId: string; | |
| }): Promise<T> { | |
| return Promise.resolve(this.mergeTokenMetadataWithCustomDataSync(params)); | |
| } | |
| @backgroundMethod() | |
| async mergeTokenMetadataWithCustomData<T extends IToken>(params: { | |
| token: T; | |
| customTokens: IAccountToken[]; | |
| networkId: string; | |
| }): Promise<T> { | |
| return this.mergeTokenMetadataWithCustomDataSync(params); | |
| } |
Summary by CodeRabbit
新功能
EnterPhase组件中添加表单提交和键盘支持功能。SearchModal组件中的搜索项logo URL构建方式。Bug 修复
文档