Skip to content

fix: block keyboard shortcuts during drag-and-drop operations#348

Merged
pengfeixx merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/block-shortcuts-during-drag
May 27, 2026
Merged

fix: block keyboard shortcuts during drag-and-drop operations#348
pengfeixx merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/block-shortcuts-during-drag

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

Expose isDragging state from ItemListView and FolderListView, disable all Shortcut elements and Delete key during drag.

拖拽记事本或文件夹时屏蔽所有快捷键操作(Delete、Ctrl+N、
Ctrl+B、F2、F3、Ctrl+R、Ctrl+S 等),避免误触。

Log: 拖拽过程中屏蔽快捷键操作
PMS: BUG-361629
Influence: 拖拽过程中所有快捷键和Delete键被禁用,松手后自动恢复。

Expose isDragging state from ItemListView and FolderListView,
disable all Shortcut elements and Delete key during drag.

拖拽记事本或文件夹时屏蔽所有快捷键操作(Delete、Ctrl+N、
Ctrl+B、F2、F3、Ctrl+R、Ctrl+S 等),避免误触。

Log: 拖拽过程中屏蔽快捷键操作
PMS: BUG-361629
Influence: 拖拽过程中所有快捷键和Delete键被禁用,松手后自动恢复。
@pengfeixx pengfeixx force-pushed the fix/block-shortcuts-during-drag branch from d91cf2e to be412e9 Compare May 27, 2026 09:15
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次代码变更的核心目的是在拖拽状态下禁用快捷键和删除操作,以防止拖拽过程中误触发其他功能

整体来看,这个修改的逻辑是清晰且合理的,但在语法逻辑、代码质量、代码性能和代码安全方面,我有一些改进建议供你参考:

一、 语法与逻辑

  1. 拖拽状态同步的潜在逻辑漏洞

    • 问题:在 FolderListView.qmlItemListView.qml 中,通过 onHeldChanged: root.isDragging = held 来同步拖拽状态。held 属性通常在鼠标按下时设为 true,在鼠标释放时设为 false。但是,如果用户在拖拽过程中按了 Esc 键,或者窗口失去焦点,QML 的 Drag 可能会被取消,此时 held 可能不会触发 onHeldChanged,或者触发时序存在延迟,导致 isDragging 未能及时重置为 false,从而锁死快捷键。
    • 建议:增加对拖拽取消状态的处理。可以通过监听 Drag 的附加属性或者 Keys.onPressed (Escape) 来强制重置状态。
  2. 快捷键 enabled 属性的短路逻辑优化

    • 问题:在 Shortcuts.qml 中,所有的 Shortcut 都加上了 !item.isDragging。对于 Ctrl+C/X/V 等剪贴板操作,如果用户是想在拖拽释放后立刻复制,由于 isDragging 可能存在微小的延迟未重置,可能会带来体验上的卡顿感。
    • 建议:确认 isDragging 的生命周期是否严格限制在鼠标按住拖动期间。一旦鼠标抬起,必须确保 isDragging 立即变为 false

二、 代码质量

  1. 属性名称的语义准确性

    • 问题:新增的属性名为 isDragging,但它实际上是由 held(按下并保持)驱动的。在 QML 中,drag.active 才是真正表示“正在拖动”的属性。如果用户只是长按而没有移动鼠标,heldtrue,此时 isDragging 也为 true,这在语义上有些不符。
    • 建议:如果只想在“移动”时禁用快捷键,建议绑定到 drag.active;如果长按也需要禁用,建议将属性重命名为 isHeldisInteracting,以提高代码的可读性和语义准确性。
  2. 代码冗余(DRY原则)

    • 问题:在 Shortcuts.qml 中,!item.isDragging 被重复书写了十几次,这使得代码显得臃肿,且未来如果需要增加其他全局禁用条件(如弹窗出现时),需要修改十几处。
    • 建议:在 Shortcuts.qml 中封装一个统一的 enabled 计算属性,或者利用 QML 的 Binding 优先级。
      // Shortcuts.qml
      property bool globalBlocked: item.isDragging || item.initialOnlyCreateFolder
      
      // 然后在各个 Shortcut 中:
      Shortcut {
          id: f1
          enabled: !globalBlocked // 代替 !item.isDragging && !item.initialOnlyCreateFolder
      }
  3. 版权声明修改

    • 修改了 SPDX-FileCopyrightText 的年份从 2024 到 2024-2026。请注意,如果是开源项目,预填未来的结束年份(2026)在某些开源合规审查中可能是不规范的,通常结束年份应为代码实际发布的年份。

三、 代码性能

  1. 属性绑定链的性能开销
    • 问题ItemListView.held -> ItemListView.isDragging -> MainWindow.itemListView.isDragging -> Shortcuts.isDragging -> 每一个 Shortcut.enabled。这是一条较长的属性绑定链。在 QML 引擎中,当 held 发生变化时,会触发十几个 Shortcutenabled 重新计算。
    • 建议:对于现代 QML 引擎(Qt 5.15+ / Qt 6),这种规模的绑定链开销微乎其微,无需过度优化。但如果你发现按键响应有延迟,可以考虑在 MainWindow 层面直接控制 Shortcuts 组件的 enabled,而不是深入到内部去控制每一个快捷键:
      // MainWindow.qml
      Shortcuts {
          id: shortcuts
          // 直接禁用整个 Shortcuts 组件,切断内部所有快捷键的响应
          enabled: rootWindow.active && !itemListView.isDragging && !folderListView.isDragging
      }
      这样只需计算一次,性能更优,代码也更简洁。

四、 代码安全

  1. 日志信息泄露(低风险)
    • 问题console.log("No notes available, cannot delete folder");console.log("No notes available, cannot delete");。在生产环境中,过多的 console.log 可能会暴露应用内部的状态逻辑。
    • 建议:如果是桌面端本地应用,这通常不是严重的安全问题,但仍建议在发布版本中移除或使用条件编译(QT_DEBUG)包裹调试日志,避免无谓的 IO 开销和信息暴露。

综合改进建议代码示例

基于以上分析,我建议对 Shortcuts.qml 和状态同步进行如下优化:

1. FolderListView.qml / ItemListView.qml (优化状态绑定)

// 建议使用 drag.active 替代 held,语义更准确
property bool isDragging: delegateDrag.drag.active 

// 如果必须用 held,确保有取消机制
// onHeldChanged: root.isDragging = held
// Keys.onPressed: (event) => { if (event.key === Qt.Key_Escape) root.isDragging = false; }

2. Shortcuts.qml (消除冗余代码)

Item {
    id: item

    property bool blockRecordingKey: false
    property bool isRecordingAudio: false
    property bool isDragging: false

    // 提取公共的禁用逻辑,减少重复代码
    property bool baseBlocked: item.isDragging || item.initialOnlyCreateFolder

    signal copy
    signal createFolder
    // ... 其他 signal

    Shortcut {
        id: f1
        autoRepeat: false
        // 使用 baseBlocked 替代重复的 &&
        enabled: !item.baseBlocked 
        sequence: "F1"
        onActivated: { showHelpManual(); }
    }

    Shortcut {
        id: ctrl_Shift_H
        enabled: !item.baseBlocked
        sequence: "Ctrl+Shift+/"
        onActivated: { showShortcutsView(); }
    }

    Shortcut {
        id: ctrl_N
        sequence: "Ctrl+N"
        // 对于包含其他特殊逻辑的,继续追加
        enabled: !item.baseBlocked && !item.blockCreateKeys
        onActivated: { createFolder(); }
    }
    // ... 其他同理
}

3. MainWindow.qml (更极致的性能与简洁方案,可选)
如果业务逻辑允许,直接在父组件禁用整个快捷键模块:

Shortcuts {
    id: shortcuts
    // 直接控制组件级别的 enabled,性能最高,代码最简
    enabled: rootWindow.active && !itemListView.isDragging && !folderListView.isDragging
    
    // 内部的各个 Shortcut 就不需要再判断 isDragging 了
    // ...
}

(注意:需确认 Shortcuts 组件内部没有其他需要在窗口非激活状态下仍需运行的逻辑,方可使用此方案)

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pengfeixx, wyu71

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

@pengfeixx pengfeixx merged commit d17ff5c into linuxdeepin:develop/snipe May 27, 2026
19 of 21 checks passed
@pengfeixx pengfeixx deleted the fix/block-shortcuts-during-drag branch May 27, 2026 09:34
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