-
Notifications
You must be signed in to change notification settings - Fork 141
fix: Fix no prompt when input is too long #2850
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
Conversation
Reviewer's GuideRefactors the authentication name TextInput into a styled container with real-time validation and alert feedback, adding length and emptiness checks while adjusting focus behavior and tooltip targeting. Sequence diagram for real-time name validation and alert displaysequenceDiagram
actor User
participant AuthenticationUI
participant textInputBackground
participant textInputItem
participant alert
participant layout
User->>AuthenticationUI: Click existing name
AuthenticationUI->>textInputItem: Set focus true, readOnly false
loop While user types
User->>textInputItem: Edit text
textInputItem->>textInputItem: onTextEdited
textInputItem->>textInputItem: Filter invalid characters
alt Length > maxLength
textInputItem->>alert: show(No more than 15 characters)
textInputItem->>textInputItem: Truncate to maxLength
end
textInputItem->>textInputItem: Update text if changed
end
User->>textInputItem: Press Enter/Return or lose focus
textInputItem->>textInputItem: onEditingFinished
textInputItem->>textInputItem: checkInputInvalid()
alt Input invalid
alt Empty
textInputItem->>alert: show(The name cannot be empty)
else Invalid chars and over length
textInputItem->>alert: show(Use letters, numbers and underscores only, and no more than 15 characters)
else Invalid chars only
textInputItem->>alert: show(Use letters, numbers and underscores only)
else Over length only
textInputItem->>alert: show(No more than 15 characters)
else Duplicate name
textInputItem->>alert: show(This name already exists)
end
textInputItem->>textInputItem: text = modelData
textInputItem->>textInputItem: Rebind text to modelData
else Input valid
textInputItem->>layout: requestRename(modelData, text)
textInputItem->>textInputItem: Rebind text to modelData
end
textInputItem->>textInputItem: focus = false
textInputItem->>textInputItem: onFocusChanged -> readOnly true
note over alert,textInputBackground: alert.target changed from layout to textInputBackground
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- In
checkInputInvalid(), theisValidcalculation mixestextandtextInputItem.textand treats empty text as valid while separately handlingisEmpty; consider normalizing to a single source (text) and simplifying the logic so validity and emptiness are not checked in overlapping conditions. - The
alertBackgroundColorproperty ontextInputBackgroundis defined but not used, whilecoloris set fromD.ColorSelector.alertBackgroundColor; either wire the property intocoloror remove the unused property to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `checkInputInvalid()`, the `isValid` calculation mixes `text` and `textInputItem.text` and treats empty text as valid while separately handling `isEmpty`; consider normalizing to a single source (`text`) and simplifying the logic so validity and emptiness are not checked in overlapping conditions.
- The `alertBackgroundColor` property on `textInputBackground` is defined but not used, while `color` is set from `D.ColorSelector.alertBackgroundColor`; either wire the property into `color` or remove the unused property to avoid confusion.
## Individual Comments
### Comment 1
<location> `src/plugin-authentication/qml/authenticationMain.qml:146` </location>
<code_context>
- text: modelData
+ Rectangle {
+ id: textInputBackground
+ property D.Palette alertBackgroundColor: DS.Style.edit.alertBackground
Layout.leftMargin: 10
Layout.preferredHeight: DS.Style.itemDelegate.height
</code_context>
<issue_to_address>
**suggestion:** The `alertBackgroundColor` property is defined but never used, which adds unnecessary noise.
`alertBackgroundColor` is declared on the `Rectangle`, but the `color` binding uses `D.ColorSelector.alertBackgroundColor` instead. Either bind `color` to this property (for configurability), e.g. `color: alert.visible ? alertBackgroundColor : "transparent"`, or remove the unused property to avoid confusion.
Suggested implementation:
```
Rectangle {
id: textInputBackground
property D.Palette alertBackgroundColor: DS.Style.edit.alertBackground
Layout.leftMargin: 10
Layout.preferredHeight: DS.Style.itemDelegate.height
color: alert.visible ? alertBackgroundColor : "transparent"
```
If there is already an existing `color` binding inside this `Rectangle` (for example `color: alert.visible ? D.ColorSelector.alertBackgroundColor : "transparent"`), you should **replace** that line instead of adding a new `color` binding. In that case, the edit should be:
- `color: alert.visible ? D.ColorSelector.alertBackgroundColor : "transparent"`
⇒ `color: alert.visible ? alertBackgroundColor : "transparent"`.
Ensure there is only one `color:` binding in this `Rectangle` block.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, pengfeixx The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fix no prompt when input is too long Log: Fix no prompt when input is too long pms: BUG-341977
|
/forcemerge |
deepin pr auto review我来对这段代码进行详细审查:
Rectangle {
id: textInputBackground
property D.Palette alertBackgroundColor: DS.Style.edit.alertBackground
Layout.leftMargin: 10
Layout.preferredHeight: DS.Style.itemDelegate.height
Layout.maximumWidth: 200
Layout.minimumWidth: 100 // 设置固定最小值
radius: 4
color: alert.visible ? D.ColorSelector.alertBackgroundColor : "transparent"
// 定义正则表达式常量
readonly property var validNameRegex: /^[A-Za-z0-9\u4e00-\u9fa5_]+$/
// 缓存名称列表
property var nameList: []
Component.onCompleted: {
updateNameList()
}
function updateNameList() {
switch (itemRep.authType) {
case CharaMangerModel.Type_Face:
nameList = dccData.model.facesList;
break;
case CharaMangerModel.Type_Finger:
nameList = dccData.model.thumbsList;
break;
case CharaMangerModel.Type_Iris:
nameList = dccData.model.irisList;
break;
}
}
TextInput {
id: textInputItem
property int maxLength: 15
anchors.fill: parent
anchors.leftMargin: 8
anchors.rightMargin: 8
text: modelData
verticalAlignment: Text.AlignVCenter
horizontalAlignment: Text.AlignLeft
focus: false
wrapMode: Text.NoWrap
readOnly: true
focusPolicy: Qt.NoFocus
color: palette.text
clip: true
selectByMouse: true
onTextEdited: {
var filteredText = filterInvalidChars(text);
if (filteredText.length > maxLength) {
alert.show(qsTr("No more than 15 characters"));
filteredText = filteredText.slice(0, maxLength);
}
if (text !== filteredText) {
text = filteredText;
}
}
function filterInvalidChars(input) {
// 使用字符白名单进行过滤
var result = "";
for (var i = 0; i < input.length; i++) {
var char = input[i];
if (validNameRegex.test(char)) {
result += char;
}
}
return result;
}
onEditingFinished: {
if (!validateInput()) {
text = modelData;
textInputItem.text = Qt.binding(function() {
return modelData
})
return;
}
focus = false;
if (modelData !== textInputItem.text) {
layout.requestRename(modelData, text);
}
textInputItem.text = Qt.binding(function() {
return modelData
})
}
function validateInput() {
if (text.length === 0) {
alert.show(qsTr("The name cannot be empty"));
return false;
}
if (!validNameRegex.test(text)) {
alert.show(qsTr("Use letters, numbers and underscores only"));
return false;
}
if (text.length > maxLength) {
alert.show(qsTr("No more than 15 characters"));
return false;
}
if (nameList.includes(text) && text !== modelData) {
alert.show(qsTr("This name already exists"));
return false;
}
return true;
}
}
}这些改进主要关注了:
|
|
This pr force merged! (status: blocked) |
Fix no prompt when input is too long
Log: Fix no prompt when input is too long
pms: BUG-341977
Summary by Sourcery
Validate and constrain authentication name input to prevent missing prompts when the entered text exceeds the allowed length.
Bug Fixes:
Enhancements: