Skip to content

fix: Optimize tab focus navigation in Remote Management#527

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:test0426
Jun 15, 2026
Merged

fix: Optimize tab focus navigation in Remote Management#527
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:test0426

Conversation

@JWWTSL

@JWWTSL JWWTSL commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

log: setCurrentIndex(...) is called synchronously within the focusOut slot, which triggers setFocus() on the newly selected ItemWidget. Although Qt's focus transfer doesn't actually complete until after focusOutEvent finishes, calling setFocus() synchronously inside a slot is an established pattern already used by existing lambdas in this codebase (the original code also directly called m_pushButton->setFocus() in the slot), so it should be stable. If focus jittering or recursion occurs during testing, fall back to using QMetaObject::invokeMethod(..., Qt::QueuedConnection) for deferred execution.

pms: bug-283743

lzwind
lzwind previously approved these changes Apr 29, 2026
@deepin-bot

deepin-bot Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 6.5.34
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #530

@deepin-bot

deepin-bot Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 6.5.35
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #534

@deepin-bot

deepin-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 6.5.36
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #537

@deepin-bot

deepin-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

TAG Bot

New tag: 6.5.37
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #541

@JWWTSL JWWTSL changed the title Fixed: Title bar theme issue in Thor Mode fix: Optimize tab focus navigation in Remote Management Jun 15, 2026
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL, lzwind

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

log: setCurrentIndex(...) is called synchronously within the focusOut slot, which triggers setFocus() on the newly selected ItemWidget. Although Qt's focus transfer doesn't actually complete until after focusOutEvent finishes, calling setFocus() synchronously inside a slot is an established pattern already used by existing lambdas in this codebase (the original code also directly called m_pushButton->setFocus() in the slot), so it should be stable. If focus jittering or recursion occurs during testing, fall back to using QMetaObject::invokeMethod(..., Qt::QueuedConnection) for deferred execution.

pms: bug-283743
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:85分

■ 【总体评价】

代码实现了跳过不可交互标签的键盘焦点精准导航,但存在明显的代码重复问题
逻辑基本正确但因四个焦点索引查找函数存在高度重复扣15分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

具体分析内容:listview.cpp中新增的四个索引查找函数边界处理合理,返回-1表示未找到。remotemanagementpanel.cpp中的信号处理逻辑分支清晰,能正确拦截并重定向焦点。
潜在问题:在setFocusInPanel和focusInEvent中,当firstFocusableIndex()返回-1(列表为空或全为不可聚焦项)时,fallback逻辑为setCurrentIndex(0),若列表为空可能导致无效的索引设置操作。
建议:当idx小于0时,应直接跳过设置列表焦点,将焦点转移至其他可用控件(如添加按钮),而不是尝试设置索引0。

  • 2.代码质量(一般)✕

具体分析内容:listview.cpp中新增的firstFocusableIndex、lastFocusableIndex、nextFocusableIndex、prevFocusableIndex四个函数,其核心逻辑高度相似,均为遍历列表并调用isFocusableItemType进行判断,仅遍历的起始点和步长不同,存在明显的代码坏味道。
潜在问题:增加了后续维护的成本,若判断逻辑(如增加新的不可聚焦类型)发生变更,需要同步修改四处代码,容易遗漏。
建议:提取一个通用的私有辅助函数,例如findFocusableIndex(int start, int end, int step),由这四个公开函数调用该辅助函数并传入不同的起始、结束和步长参数,以消除重复。

  • 3.代码性能(无性能问题)✓

具体分析内容:新增的四个查找函数时间复杂度均为O(N),但在GUI列表焦点切换场景下,列表项数量通常较少,线性遍历的开销可以忽略不计,不会引起界面卡顿。
建议:保持现有实现即可。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码仅涉及UI组件的索引查找和焦点设置,使用at()方法安全访问列表元素,未引入任何外部输入处理、命令执行或内存越界风险。
建议:无需额外安全修复。

■ 【改进建议代码示例】

// listview.cpp:消除重复代码的优化实现
static bool isFocusableItemType(ItemFuncType t)
{
    return t != ItemFuncType_GroupLabel && t != ItemFuncType_ItemLabel;
}

// 提取通用查找逻辑
int ListView::findFocusableIndex(int start, int end, int step)
{
    for (int i = start; i != end + step; i += step) {
        if (i >= 0 && i < m_itemList.count() && isFocusableItemType(m_itemList.at(i)->getFuncType())) {
            return i;
        }
    }
    return -1;
}

int ListView::firstFocusableIndex()
{
    return m_itemList.isEmpty() ? -1 : findFocusableIndex(0, m_itemList.count() - 1, 1);
}

int ListView::lastFocusableIndex()
{
    return m_itemList.isEmpty() ? -1 : findFocusableIndex(m_itemList.count() - 1, 0, -1);
}

int ListView::nextFocusableIndex(int from)
{
    return findFocusableIndex(from + 1, m_itemList.count() - 1, 1);
}

int ListView::prevFocusableIndex(int from)
{
    return findFocusableIndex(from - 1, 0, -1);
}
// remotemanagementpanel.cpp:修复边界fallback逻辑
void RemoteManagementPanel::setFocusInPanel()
{
    if (m_listWidget->isVisible()) {
        qCDebug(remotemanage) << "list widget is visible, setting focus";
        /******** Modify by ut000610 daizhengwen 2020-07-27:bug#39775 Begin***************/
        // 将焦点设置在列表里的第一个可聚焦项
        int idx = m_listWidget->firstFocusableIndex();
        if (idx >= 0) {
            m_listWidget->setCurrentIndex(idx);
        } else {
            // 列表为空或全为不可聚焦项时,直接将焦点设置到添加按钮
            m_pushButton->setFocus();
        }
        /********************* Modify by ut000610 daizhengwen End ************************/
    } else {
        qCDebug(remotemanage) << "setting focus on add button";
        m_pushButton->setFocus();
    }
}

@JWWTSL

JWWTSL commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit 8773dc1 into linuxdeepin:master Jun 15, 2026
17 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