fix(accounts): fix IME input and highlight color in password field#3302
Conversation
|
Hi @MyLeeJiEun. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts password TextInput highlight color initialization and conditionally sets input method hints based on password visibility to fix IME behavior and prevent a wrong-color flash. Flow diagram for conditional password inputMethodHints and echoModeflowchart TD
A[Password TextInput created] --> B[normalHighlight = palette.highlight]
B --> C{echoButtonVisible?}
C -->|true| D[echoMode = TextInput.Password]
C -->|true| E[inputMethodHints = Qt.ImhHiddenText | Qt.ImhNoPredictiveText | Qt.ImhNoAutoUppercase]
C -->|false| F[echoMode = TextInput.Normal]
C -->|false| G[inputMethodHints = Qt.ImhNone]
H[hasErrorBorder or showAlert] --> I[palette.highlight = #FF5736]
H --> J[palette.highlight = normalHighlight]
File-Level Changes
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:
- By binding
normalHighlightdirectly topalette.highlightinstead of caching it onComponent.onCompleted, you may lose the ability to restore the original focus color if the palette changes dynamically at runtime; consider keeping a one-time cached value if that behavior is still required. - Using
echoButtonVisibleto switchinputMethodHintsmay not exactly reflect the actual echo state (password hidden vs. shown); consider basing the hints onechoModeor a dedicated visibility flag to avoid mismatches when the button is visible but the password is currently plain text.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By binding `normalHighlight` directly to `palette.highlight` instead of caching it on `Component.onCompleted`, you may lose the ability to restore the original focus color if the palette changes dynamically at runtime; consider keeping a one-time cached value if that behavior is still required.
- Using `echoButtonVisible` to switch `inputMethodHints` may not exactly reflect the actual echo state (password hidden vs. shown); consider basing the hints on `echoMode` or a dedicated visibility flag to avoid mismatches when the button is visible but the password is currently plain text.
## Individual Comments
### Comment 1
<location path="src/plugin-accounts/qml/PasswordLayout.qml" line_range="511-512" />
<code_context>
// Cache the normal focus color so we can restore it.
- property color normalHighlight: "transparent"
- Component.onCompleted: normalHighlight = palette.highlight
+ property color normalHighlight: palette.highlight
palette.highlight: (hasErrorBorder || showAlert) ? "#FF5736" : normalHighlight
topPadding: 0
bottomPadding: 0
</code_context>
<issue_to_address>
**issue (bug_risk):** This change introduces a binding loop between `normalHighlight` and `palette.highlight`.
`normalHighlight` now depends on `palette.highlight`, while `palette.highlight` also depends on `normalHighlight`, creating a circular binding that will trigger QML binding loop warnings and may cause unstable behavior. The earlier `Component.onCompleted` assignment avoided this by copying the initial value once. To fix this, either keep `normalHighlight` as a one-shot cached value (e.g., set once on completion) or refactor so `palette.highlight` no longer references `normalHighlight` (for example, cache the base highlight in a separate non-reactive property).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Use Qt.binding() for palette.highlight to keep color reactive when hasErrorBorder or showAlert state changes 2. Conditionally apply inputMethodHints based on echoButtonVisible so input method works correctly when password is visible Log: Fix password input field highlight color binding and input method hints in accounts module fix(accounts): 修复密码输入框绑定和输入法提示问题 1. 使用 Qt.binding() 绑定 palette.highlight,确保高亮色随 hasErrorBorder 或 showAlert 状态变化正确更新 2. 根据 echoButtonVisible 动态设置 inputMethodHints, 使密码可见时输入法能正常工作 Log: 修复账户模块密码输入框高亮色绑定失效及输入法提示异常问题 PMS: BUG-366507
484f4ba to
f567b1c
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, MyLeeJiEun The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Log: Fix Chinese IME input not working in password field when PIN is visible
fix(accounts): 修复密码框输入法和高亮颜色问题
Log: 修复密码明文显示时无法使用中文输入法的问题
PMS: BUG-366507
Summary by Sourcery
Fix password field behavior for highlight color and input method handling when toggling visibility.
Bug Fixes: