fix(slider): adjust path calculation for handle alignment#617
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom May 8, 2026
Merged
fix(slider): adjust path calculation for handle alignment#617deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Contributor
|
Hi @52cyb. 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 slider path and filled track calculations so that when the handle type is NoArrowType (no visible handle), the path endpoints and passItem width are aligned based on the groove dimensions and visualPosition instead of the (non‑existent) handle geometry. Updated class diagram for Slider handleType and geometry calculationclassDiagram
class Slider {
bool horizontal
HandleType handleType
real visualPosition
Handle handle
Groove sliderGroove
PassItem passItem
}
class HandleType {
<<enumeration>>
ArrowUp
ArrowLeft
ArrowBottom
ArrowRight
NoArrowType
}
class Handle {
real x
real y
real width
real height
}
class Groove {
real width
real height
}
class PassItem {
real x
real y
real width
real height
}
Slider --> HandleType : uses
Slider *-- Handle : contains
Slider *-- Groove : contains
Slider *-- PassItem : contains
Flow diagram for slider path and passItem width calculation with NoArrowTypeflowchart TD
A[Start layout of slider path and passItem] --> B{horizontal?}
B -->|yes| C{handleType is NoArrowType?}
C -->|yes| D[PathLine.x = visualPosition * sliderGroove.width]
C -->|no| E[PathLine.x = handle.x]
D --> F[passItem.width = visualPosition * sliderGroove.width]
E --> F[passItem.width = handle.x]
B -->|no| G{handleType is NoArrowType?}
G -->|yes| H[PathLine.y = visualPosition * sliderGroove.height]
G -->|no| I[PathLine.y = handle.y + handle.height / 2]
F --> J[PathLine.y = sliderGroove.height / 2 for horizontal]
H --> K[passItem.width = DS_Style_slider_groove_height]
I --> K
J --> L[End layout]
K --> L
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
NoArrowTypeenum value is added to theArrowTypeenumeration but used asSlider.HandleType.NoArrowType; please double‑check the enum/namespace consistency so that the type name and usage match and avoid confusion or runtime issues. - The repeated inline ternary checks on
control.handleType === Slider.HandleType.NoArrowTypeforx,y, andpassItem.widthmake the path geometry harder to read; consider extracting a small helper or intermediate properties (e.g.,effectiveHandleX/Y) to centralize this logic and improve maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `NoArrowType` enum value is added to the `ArrowType` enumeration but used as `Slider.HandleType.NoArrowType`; please double‑check the enum/namespace consistency so that the type name and usage match and avoid confusion or runtime issues.
- The repeated inline ternary checks on `control.handleType === Slider.HandleType.NoArrowType` for `x`, `y`, and `passItem.width` make the path geometry harder to read; consider extracting a small helper or intermediate properties (e.g., `effectiveHandleX/Y`) to centralize this logic and improve maintainability.
## Individual Comments
### Comment 1
<location path="qt6/src/qml/Slider.qml" line_range="21" />
<code_context>
ArrowBottom = 4,
- ArrowRight = 5
+ ArrowRight = 5,
+ NoArrowType = 6
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider how the new `NoArrowType` integrates with any existing assumptions about handle types.
This new enum value breaks the prior invariant that all handle types are arrow-shaped, so any logic that assumes `handleType` always implies an arrow might now behave incorrectly. Please review callers that switch/branch on `handleType` to ensure `NoArrowType` is handled explicitly rather than falling into an unintended default/fallback path.
</issue_to_address>
### Comment 2
<location path="qt6/src/qml/Slider.qml" line_range="118-120" />
<code_context>
y: control.horizontal ? -DS.Style.slider.groove.height / 2 : control.handle.y + control.handle.height / 2
height: control.horizontal ? DS.Style.slider.groove.height : sliderGroove.height - control.handle.y - control.handle.height / 2
- width: control.horizontal ? control.handle.x : DS.Style.slider.groove.height
+ width: control.horizontal ? (control.handleType === Slider.HandleType.NoArrowType
+ ? control.visualPosition * sliderGroove.width
+ : control.handle.x) : DS.Style.slider.groove.height
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Vertical `passItem` still uses `handle.y` while the path now uses `visualPosition` for `NoArrowType`, which can desync the visual elements.
In the vertical case, `PathLine.y` now uses `visualPosition * sliderGroove.height` for `NoArrowType`, but `passItem.y` / `height` still use `control.handle.y` / `height`. If `NoArrowType` changes how `handle.y` is derived or leaves it meaningless, the passed region can drift from the knob position. Consider updating the vertical `passItem` bindings for `NoArrowType` to follow the same `visualPosition`-based logic as the path endpoint so all visuals stay aligned.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When handleType is NoHandleType, the x-coordinate calculation now calculates based on sliderGroove to correct the visual alignment of the slider path. Log: adjust path calculation for handle alignment PMS: BUG-358623 Influence: 检查输入设备的反馈音量显示是否正确，同时关注静音场景（PMS:350837）
18202781743
approved these changes
May 8, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, 52cyb 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 |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When handleType is NoHandleType, the x-coordinate calculation now calculates based on sliderGroove to correct the visual alignment of the slider path.
Log: adjust path calculation for handle alignment
PMS: BUG-358623
Influence: 检查输入设备的反馈音量显示是否正确，同时关注静音场景（PMS:350837）
Summary by Sourcery
Adjust slider path and pass region calculations to keep the filled track visually aligned when using a handle-less slider variant.
Bug Fixes:
Enhancements: