Skip to content

fix: Fix the issue where tab cannot select focus#2900

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix-311339
Dec 24, 2025
Merged

fix: Fix the issue where tab cannot select focus#2900
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:fix-311339

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Dec 24, 2025

Copy link
Copy Markdown
Contributor

Fix the issue where tab cannot select focus

Log: Fix the issue where tab cannot select focus
pms: BUG-311339

Summary by Sourcery

Improve keyboard focus handling for control center items and dock plugin area pages.

Bug Fixes:

  • Allow control center list items to receive focus via the Tab key and navigate with Up/Down keys, including activating associated right-side controls with Enter/Return.
  • Prevent specific action buttons and the plugin area parent item from being focusable via Tab where inappropriate to avoid incorrect focus behavior.

Fix the issue where tab cannot select focus

Log: Fix the issue where tab cannot select focus
pms: BUG-311339
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

我来对这个git diff进行详细的代码审查:

  1. 代码逻辑审查:
  • 在DccCheckIcon.qml中添加了activeFocusOnTab: false,这确保了该按钮不会被Tab键选中,这对于装饰性图标是合理的。
  • 在DccEditorItem.qml中添加了键盘导航支持,包括上下键导航和回车键触发点击事件,这提高了可访问性。
  • 在DccItem.qml中添加了activeFocusOnTab: true,使项目可以通过Tab键获得焦点。
  • 在PluginArea.qml中添加了onParentItemChanged处理,确保父项不会获得焦点。
  1. 代码质量改进建议:
  • DccEditorItem.qml中的键盘事件处理可以进一步优化:
Keys.onUpPressed: {
    const prevItem = nextItemInFocusChain(false)
    if (prevItem) prevItem.forceActiveFocus(Qt.BacktabFocusReason)
}
Keys.onDownPressed: {
    const nextItem = nextItemInFocusChain(true)
    if (nextItem) nextItem.forceActiveFocus(Qt.TabFocusReason)
}

这样可以避免可选链操作符可能带来的兼容性问题。

  1. 性能考虑:
  • 当前的实现基本不会带来性能问题,因为焦点控制是轻量级操作。
  • 建议在DccEditorItem.qml中添加防抖处理,避免快速按键时的性能消耗:
property bool keyProcessing: false

Keys.onUpPressed: {
    if (keyProcessing) return
    keyProcessing = true
    const prevItem = nextItemInFocusChain(false)
    if (prevItem) prevItem.forceActiveFocus(Qt.BacktabFocusReason)
    keyProcessingTimer.restart()
}

Timer {
    id: keyProcessingTimer
    interval: 50
    onTriggered: keyProcessing = false
}
  1. 安全性考虑:
  • 确保所有焦点相关的操作都在UI线程中执行,避免跨线程操作。
  • 在处理键盘事件时,应该验证目标项的有效性:
Keys.onReturnPressed: {
    if (rightLoader.item && typeof rightLoader.item.clicked === 'function') {
        rightLoader.item.clicked()
    }
}
  1. 可访问性改进建议:
  • 为获得焦点的项目添加适当的提示信息:
Accessible.name: model.item.name
Accessible.description: model.item.description
  • 在焦点变化时添加适当的视觉反馈:
onActiveFocusChanged: {
    if (activeFocus) {
        // 添加视觉反馈
    }
}
  1. 其他建议:
  • 考虑添加焦点顺序的自定义配置,允许用户调整导航顺序。
  • 可以添加焦点相关的单元测试,确保焦点管理在各种场景下都能正常工作。

总体来说,这些改动提高了应用的可访问性,使键盘用户能够更好地导航和使用界面。建议在实施时进行充分的测试,确保所有交互场景都能正常工作。

@sourcery-ai

sourcery-ai Bot commented Dec 24, 2025

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Enables keyboard Tab/arrow navigation and activation within control center item delegates while preventing focus from landing on icon-only controls that shouldn’t be tabbable, and ensures the dock plugin area page’s parent item is not tabbable by default.

Sequence diagram for key event handling in DccEditorItem focus navigation

sequenceDiagram
  actor User
  participant DccEditorItem
  participant FocusChain
  participant RightLoader
  participant RightLoaderItem

  User->>DccEditorItem: Press_Tab
  DccEditorItem->>FocusChain: nextItemInFocusChain(true)
  FocusChain-->>DccEditorItem: nextItem
  DccEditorItem->>FocusChain: nextItem.forceActiveFocus(Qt.TabFocusReason)

  User->>DccEditorItem: Press_Up
  DccEditorItem->>FocusChain: nextItemInFocusChain(false)
  FocusChain-->>DccEditorItem: previousItem
  DccEditorItem->>FocusChain: previousItem.forceActiveFocus(Qt.BacktabFocusReason)

  User->>DccEditorItem: Press_Return_or_Enter
  DccEditorItem->>RightLoader: access_item
  alt RightLoader_has_item
    RightLoader->>RightLoaderItem: clicked()
  else RightLoader_item_missing
    RightLoader-->>DccEditorItem: no_action
  end
Loading

Flow diagram for focus eligibility and tab behavior in control center and dock plugin area

flowchart TD
  A_Start[Start key navigation] --> B_CheckItemType{Is focused item a DccEditorItem or DccItem?}

  B_CheckItemType -->|Yes| C_FocusAllowed[activeFocusOnTab true]
  B_CheckItemType -->|No| D_CheckIconOrParent{Is item DccCheckIcon or PluginArea_parent_item?}

  D_CheckIconOrParent -->|Yes| E_FocusBlocked[activeFocusOnTab false]
  D_CheckIconOrParent -->|No| F_DefaultFocusBehavior[Use_default_QML_focus_behavior]

  C_FocusAllowed --> G_KeyPressed{Key pressed?}
  G_KeyPressed -->|Tab_or_Down| H_MoveNext[nextItemInFocusChain true and forceActiveFocus Qt_TabFocusReason]
  G_KeyPressed -->|Backtab_or_Up| I_MovePrevious[nextItemInFocusChain false and forceActiveFocus Qt_BacktabFocusReason]
  G_KeyPressed -->|Enter_or_Return| J_TryActivateRightLoader{Does rightLoader_have_item?}

  J_TryActivateRightLoader -->|Yes| K_InvokeClick[rightLoader.item.clicked]
  J_TryActivateRightLoader -->|No| L_NoAction[No action]

  H_MoveNext --> M_End[End key handling]
  I_MovePrevious --> M_End
  K_InvokeClick --> M_End
  L_NoAction --> M_End
  E_FocusBlocked --> M_End
  F_DefaultFocusBehavior --> M_End
Loading

File-Level Changes

Change Details Files
Make DccEditorItem and DccItem delegates keyboard-focusable via Tab and add keyboard handlers for moving focus and activating the right-side control.
  • Enable activeFocusOnTab on item delegates so they participate in the Tab focus chain.
  • Handle Up/Down arrow keys to move focus to the previous/next item in the focus chain with appropriate focus reasons.
  • Handle Enter/Return keys to invoke the right-side loader’s clicked handler when present by delegating to rightLoader.item.clicked().
  • Introduce an id for the right-side DccLoader to reference it from key handlers.
src/dde-control-center/frame/plugin/DccEditorItem.qml
src/dde-control-center/frame/plugin/DccItem.qml
Prevent non-interactive or icon-only controls from stealing keyboard tab focus.
  • Explicitly disable tab focus on the check icon action button by setting activeFocusOnTab to false.
  • Ensure the PluginArea’s parent item is not tabbable by default by clearing activeFocusOnTab when the parent item changes.
src/dde-control-center/frame/plugin/DccCheckIcon.qml
src/plugin-dock/qml/PluginArea.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The use of optional chaining in nextItemInFocusChain(false)?.forceActiveFocus(...) may not be supported by all QML/JS versions; consider using an explicit null check before calling forceActiveFocus to avoid runtime issues.
  • In the Keys.onReturnPressed/Keys.onEnterPressed handlers, guard not only against rightLoader.item being null but also ensure that rightLoader.item.clicked actually exists before invoking it, to avoid potential runtime errors when the loaded item lacks a clicked handler.
  • If the intent is for the Up/Down key handlers to fully control focus navigation, consider explicitly accepting the key events (e.g., via the event parameter) to prevent unintended propagation and duplicate focus changes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The use of optional chaining in `nextItemInFocusChain(false)?.forceActiveFocus(...)` may not be supported by all QML/JS versions; consider using an explicit null check before calling `forceActiveFocus` to avoid runtime issues.
- In the `Keys.onReturnPressed`/`Keys.onEnterPressed` handlers, guard not only against `rightLoader.item` being null but also ensure that `rightLoader.item.clicked` actually exists before invoking it, to avoid potential runtime errors when the loaded item lacks a `clicked` handler.
- If the intent is for the Up/Down key handlers to fully control focus navigation, consider explicitly accepting the key events (e.g., via the event parameter) to prevent unintended propagation and duplicate focus changes.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot

Copy link
Copy Markdown

[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.

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

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Dec 24, 2025

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit c67c55d into linuxdeepin:master Dec 24, 2025
16 of 18 checks passed
@pengfeixx pengfeixx deleted the fix-311339 branch December 24, 2025 03:05
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