Skip to content

fix(slider): adjust path calculation for handle alignment#564

Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom
add-uos:master
Feb 5, 2026
Merged

fix(slider): adjust path calculation for handle alignment#564
18202781743 merged 1 commit intolinuxdeepin:masterfrom
add-uos:master

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Feb 5, 2026

When handleType is less than 0, the x-coordinate calculation now includes the handle width offset to correct the visual alignment of the slider path.

log: adjust path calculation for handle alignment
pms: BUG-349793

When handleType is less than 0, the x-coordinate calculation now includes the handle width offset to correct the visual alignment of the slider path.

log: adjust path calculation for handle alignment
pms: BUG-349793
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @add-uos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码修改主要是针对 PathLinex 属性进行了条件判断的扩展。以下是对这段代码的详细审查意见,涵盖语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 逻辑分析
    • 原代码x: control.horizontal ? control.handle.x : sliderGroove.width / 2
      • 当滑块水平时,路径终点 x 坐标直接取 handle.x
    • 新代码x: control.horizontal ? (control.handleType < 0 ? control.handle.x + control.handle.width : control.handle.x) : sliderGroove.width / 2
      • 引入了 control.handleType 的判断。
      • 如果 control.handleType < 0,则 x 坐标为 control.handle.x + control.handle.width(即滑块的右边缘)。
      • 否则,保持原逻辑 control.handle.x(即滑块的左边缘)。
  • 潜在问题
    • 变量定义:代码中引入了 control.handleType,但在这个 diff 中并未看到其定义或绑定。需要确保该属性在 T.Slider 或其父组件中已正确定义,否则会导致运行时错误 "ReferenceError: control.handleType is not defined"。
    • 语义清晰度:使用 < 0 来判断类型显得有些"魔数"(Magic Number)。通常枚举值或布尔值(如 handleAlignRight)会更易读。如果 handleType 是一个枚举,建议使用具名常量进行比较(例如 Slider.HandleType.Right)。

2. 代码质量

  • 可读性
    • 三元运算符嵌套使得单行代码变长,虽然 QML 支持这种写法,但降低了可读性。
    • 建议:如果逻辑允许,可以将计算逻辑提取为一个单独的属性或函数,或者通过代码格式化(换行)来提高可读性。
  • 一致性
    • y 属性的计算中:y: control.horizontal ? sliderGroove.height / 2 : control.handle.y + control.handle.height / 2
    • 在垂直模式下,y 坐标固定取 handle.y + handle.height / 2(滑块中心)。
    • 改进建议:如果 handleType 的意图是改变滑块的对齐方式(如左对齐/右对齐或上对齐/下对齐),那么垂直模式下的 y 坐标计算逻辑是否也应该受到 handleType 的影响?目前的修改只影响了水平模式,这可能是不对称的,取决于具体的设计需求。

3. 代码性能

  • 计算开销
    • 增加了一个额外的属性访问(control.handleType)和一个比较操作。这在 QML 的属性绑定系统中是非常微小的开销,通常可以忽略不计。
  • 绑定触发频率
    • 由于 PathLine 位于 Path 元素中,通常只有在 handle 移动(拖动)或 control.horizontal 改变时才会重新计算。只要 control.handleType 不是频繁变化的属性,性能影响极小。

4. 代码安全

  • 空值检查
    • 代码访问了 control.handle。如果在某些边缘情况下 handlenull(例如滑块尚未初始化完成),访问 handle.xhandle.width 会导致程序崩溃。
    • 建议:虽然原代码也没有做检查,但在添加新逻辑时,最好确保 handle 的有效性,例如使用 control.handle ? ... : defaultValue
  • 类型安全
    • 确认 control.handleType 的类型。如果是 intenum,比较 < 0 是合法的。但如果是其他类型(如 string),可能会导致意外的行为。

总结与改进建议

这段代码的修改旨在根据 handleType 调整滑块轨道填充路径的终点位置。虽然功能上看似可行,但为了提高代码的健壮性和可维护性,建议进行以下修改:

改进后的代码示例:

PathLine {
    // 建议将复杂的逻辑提取出来,或者至少增加注释说明 handleType 的含义
    // 假设 handleType < 0 代表某种特定的对齐方式(例如右对齐)
    x: {
        if (!control.horizontal) {
            return sliderGroove.width / 2;
        }
        if (!control.handle) {
            return 0; // 安全检查,防止 handle 为空
        }
        // 使用更具语义的判断,或者定义枚举常量
        if (control.handleType < 0) {
            return control.handle.x + control.handle.width;
        }
        return control.handle.x;
    }
    y: control.horizontal ? sliderGroove.height / 2 : control.handle.y + control.handle.height / 2
}

具体建议点:

  1. 增加空值检查:防止 handle 未初始化时崩溃。
  2. 优化三元运算符:使用代码块 { ... } 替代过长的三元表达式,提高可读性。
  3. 明确常量含义:如果 < 0 有特定含义,建议定义一个清晰的枚举属性(如 handleAlignment),并在代码注释中说明。
  4. 垂直模式的一致性:确认垂直滑块是否也需要根据 handleType 调整 y 坐标的计算逻辑。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, add-uos

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@18202781743 18202781743 merged commit 95a2c37 into linuxdeepin:master Feb 5, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants