fix(settings): 防止凭据字段清空已有密钥#49
Conversation
Reviewer's GuideUpdates the settings credentials field component to be load-aware, track dirty state, debounce-safe across account changes, and provide explicit status feedback and error handling for read/save/copy operations to prevent accidentally overwriting existing secrets with empty values. Sequence diagram for credential field load and save behaviorsequenceDiagram
participant User
participant CredentialField
participant readCredential
participant setCredential
Note over CredentialField: Initial mount or account change
CredentialField->>CredentialField: setLoaded(false)
CredentialField->>CredentialField: setDirty(false)
CredentialField->>CredentialField: setStatus(idle)
CredentialField->>CredentialField: clearTimeout(debounceRef)
CredentialField->>readCredential: readCredential(account)
alt read success
readCredential-->>CredentialField: value or null
CredentialField->>CredentialField: setValue(value or empty)
CredentialField->>CredentialField: setLoaded(true)
else read failure
readCredential--xCredentialField: error
CredentialField->>CredentialField: console.error(read failure)
CredentialField->>CredentialField: setLoaded(true)
CredentialField->>CredentialField: setStatus(saveError)
end
User->>CredentialField: type in input
CredentialField->>CredentialField: setValue(v)
CredentialField->>CredentialField: setDirty(true)
CredentialField->>CredentialField: clearTimeout(debounceRef)
CredentialField->>CredentialField: debounce(save(v), 300ms)
Note over CredentialField: Debounced save
CredentialField->>setCredential: save(v) when debounce fires
alt loaded is false
setCredential-->>CredentialField: ignored (save returns)
else loaded is true
CredentialField->>CredentialField: setStatus(saving)
alt save success
setCredential-->>CredentialField: ok
CredentialField->>CredentialField: setDirty(false)
CredentialField->>CredentialField: setStatus(saved)
else save failure
setCredential--xCredentialField: error
CredentialField->>CredentialField: console.error(save failure)
CredentialField->>CredentialField: setStatus(saveError)
end
CredentialField->>CredentialField: setTimeout(setStatus(idle), 1600ms)
end
User->>CredentialField: blur input
alt not loaded or not dirty
CredentialField-->>User: no save
else loaded and dirty
CredentialField->>CredentialField: clearTimeout(debounceRef)
CredentialField->>setCredential: save(current value)
end
Sequence diagram for credential field copy behaviorsequenceDiagram
participant User
participant CredentialField
participant ClipboardAPI as navigator.clipboard
User->>CredentialField: click copy button
alt value is empty
CredentialField-->>User: do nothing
else value is not empty
CredentialField->>CredentialField: onCopy()
alt ClipboardAPI available
CredentialField->>ClipboardAPI: writeText(value)
alt copy success
ClipboardAPI-->>CredentialField: ok
CredentialField->>CredentialField: setStatus(copied)
else copy failure
ClipboardAPI--xCredentialField: error
CredentialField->>CredentialField: console.error(copy failure)
CredentialField->>CredentialField: setStatus(copyError)
end
else ClipboardAPI unavailable
CredentialField->>CredentialField: throw Error(Clipboard API unavailable)
CredentialField->>CredentialField: console.error(copy failure)
CredentialField->>CredentialField: setStatus(copyError)
end
CredentialField->>CredentialField: setTimeout(setStatus(idle), 1600ms)
end
Class diagram for updated credential field state and behaviorclassDiagram
class CredentialFieldProps {
string label
string account
string placeholder
boolean mono
boolean mask
string defaultValue
}
class CredentialFieldState {
string value
boolean revealed
boolean loaded
boolean dirty
Status status
TimeoutHandle debounceRef
}
class Status {
<<enumeration>>
idle
saving
saved
saveError
copied
copyError
}
class CredentialFieldComponent {
+CredentialFieldComponent(props)
+useEffect_loadCredential(account)
+useEffect_clearDebounceOnUnmount()
+save(value)
+handleChange(event)
+onBlur()
+fillDefault()
+onCopy()
+render()
}
class readCredentialService {
+readCredential(account) string
}
class setCredentialService {
+setCredential(account, value)
}
CredentialFieldComponent --> CredentialFieldProps : uses
CredentialFieldComponent --> CredentialFieldState : manages
CredentialFieldState --> Status : has
CredentialFieldComponent --> readCredentialService : calls
CredentialFieldComponent --> setCredentialService : calls
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new status messages ('保存中', '已复制', '操作失败') are hard-coded; consider routing these through the existing i18n
t(...)mechanism for consistency with the rest of the settings UI. - Both
saveandonCopyusewindow.setTimeoutto resetstatuswithout any cleanup; consider tracking these timeouts in a ref and clearing them in auseEffectcleanup to avoid possible state updates on an unmounted component.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new status messages ('保存中', '已复制', '操作失败') are hard-coded; consider routing these through the existing i18n `t(...)` mechanism for consistency with the rest of the settings UI.
- Both `save` and `onCopy` use `window.setTimeout` to reset `status` without any cleanup; consider tracking these timeouts in a ref and clearing them in a `useEffect` cleanup to avoid possible state updates on an unmounted component.
## Individual Comments
### Comment 1
<location path="openless-all/app/src/pages/Settings.tsx" line_range="490-495" />
<code_context>
+ whiteSpace: 'nowrap',
+ }}
+ >
+ {status === 'saving'
+ ? '保存中'
+ : status === 'saved'
+ ? t('common.saved')
+ : status === 'copied'
+ ? '已复制'
+ : '操作失败'}
+ </span>
</code_context>
<issue_to_address>
**suggestion:** Hardcoded Chinese status messages break consistency with the i18n pattern used elsewhere.
These status texts (`'保存中'`, `'已复制'`, `'操作失败'`) should also use the i18n helper instead of being hardcoded. Please define appropriate keys (e.g. under `settings.credential.*`) and call `t(...)` so status messages are localized consistently with the rest of the component.
Suggested implementation:
```typescript
{status !== 'idle' && (
<span
style={{
fontSize: 11,
color: status.endsWith('Error') ? 'var(--ol-warn)' : 'var(--ol-ok)',
whiteSpace: 'nowrap',
}}
>
{status === 'saving'
? t('settings.credential.saving')
: status === 'saved'
? t('common.saved')
: status === 'copied'
? t('settings.credential.copied')
: t('settings.credential.actionFailed')}
</span>
)}
</div>
</SettingRow>
```
To fully implement this, add the corresponding i18n keys in your locale files, for example:
- In `locales/zh/settings.json` (or equivalent):
- `"credential.saving": "保存中"`
- `"credential.copied": "已复制"`
- `"credential.actionFailed": "操作失败"`
- In other locale files (e.g. `en`), provide appropriate translations for:
- `settings.credential.saving`
- `settings.credential.copied`
- `settings.credential.actionFailed`
Also ensure that the `settings` namespace is loaded for this page if your i18n setup requires explicit namespace loading.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| {status === 'saving' | ||
| ? '保存中' | ||
| : status === 'saved' | ||
| ? t('common.saved') | ||
| : status === 'copied' | ||
| ? '已复制' |
There was a problem hiding this comment.
suggestion: Hardcoded Chinese status messages break consistency with the i18n pattern used elsewhere.
These status texts ('保存中', '已复制', '操作失败') should also use the i18n helper instead of being hardcoded. Please define appropriate keys (e.g. under settings.credential.*) and call t(...) so status messages are localized consistently with the rest of the component.
Suggested implementation:
{status !== 'idle' && (
<span
style={{
fontSize: 11,
color: status.endsWith('Error') ? 'var(--ol-warn)' : 'var(--ol-ok)',
whiteSpace: 'nowrap',
}}
>
{status === 'saving'
? t('settings.credential.saving')
: status === 'saved'
? t('common.saved')
: status === 'copied'
? t('settings.credential.copied')
: t('settings.credential.actionFailed')}
</span>
)}
</div>
</SettingRow>To fully implement this, add the corresponding i18n keys in your locale files, for example:
-
In
locales/zh/settings.json(or equivalent):"credential.saving": "保存中""credential.copied": "已复制""credential.actionFailed": "操作失败"
-
In other locale files (e.g.
en), provide appropriate translations for:settings.credential.savingsettings.credential.copiedsettings.credential.actionFailed
Also ensure that the settings namespace is loaded for this page if your i18n setup requires explicit namespace loading.
包含本轮所有合并: - Codex 终审两条 HIGH (cancel race) 修复 (PR #79) - 6 个 Cooper-X-Oak/Codex bot PRs 自动合并 (#44 #49 #53 #68 #72 #73) - 2 个有冲突 PR 本地 rebase 后合并 (#66 cancel + 空转写并存 / #67 Windows docs) - README 破图修复 (PR #80) - workflow-scope 受限的 #48 + #75 由用户在 GitHub UI 直接合并 3 处版本字段同步:package.json + tauri.conf.json + Cargo.toml
摘要
关联 fork 验证:Cooper-X-Oak#12。
本 PR 是从 fork/dev 已验证批次拆出的第三个最小 upstream 维护项:修复设置页凭据字段在读取完成前可能把已有密钥保存为空值的问题。
fork/dev 先行验证
b9ee8b8。修复 / 新增 / 改进
loaded/dirty状态:只有用户实际修改后的字段才会保存。onCopy,剪贴板 API 不可用时不再静默失败。兼容
测试计划
npm run buildgit diff --checkSummary by Sourcery
Prevent settings credential fields from overwriting existing secrets with empty values and improve user feedback for credential operations.
Bug Fixes:
Enhancements: