Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Resurgamz 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 |
Reviewer's GuideRestores normal reading-mode scrolling by removing QAction registrations that intercepted arrow keys and Space, while preserving slide-show keyboard behavior through direct focus and a keyPressEvent override. Sequence diagram for restored keyboard scrolling and slide-show handlingsequenceDiagram
actor User
participant SheetBrowser
participant SlideWidget
participant Utils
User->>SheetBrowser: keyPressEvent(event)
Note over SheetBrowser: Arrow keys and Space are no longer QAction shortcuts
SheetBrowser-->>User: Default scrolling behavior
User->>SlideWidget: keyPressEvent(event)
SlideWidget->>Utils: getKeyshortcut(event)
Utils-->>SlideWidget: key
SlideWidget->>SlideWidget: handleKeyPressEvent(key)
SlideWidget-->>User: Navigate slides or toggle autoplay
Flow diagram for keyboard event routing after shortcut removalflowchart TD
A[Keyboard input] --> B{Slide-show mode?}
B -- No --> C[SheetBrowser keyPressEvent]
C --> D[Qt default scrolling]
B -- Yes --> E[SlideWidget receives focus]
E --> F[SlideWidget keyPressEvent]
F --> G[Utils.getKeyshortcut]
G --> H[SlideWidget.handleKeyPressEvent]
H --> I[Slide navigation or autoplay control]
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}
… registration Arrow keys (Left/Right/Up/Down) and Space were registered as QAction shortcuts in Central.cpp constructor, which intercepted key events and prevented Qt's default scrolling behavior in document reading mode. The page navigation logic in CentralDocPage.cpp for these keys was already commented out, so the QAction had no effect except blocking default scrolling. Fix by commenting out the QAction registrations for these keys in Central.cpp, removing the slide widget key forwarding in CentralDocPage.cpp, and adding a keyPressEvent override to SlideWidget with setFocus() in its constructor so slide show mode handles arrow keys and space independently. This restores the fix from commit a81e5e1 that was reverted by the v23 merge commit b20fcf9. Log: 方向键无法滚动文档页面 Bug: https://pms.uniontech.com/bug-view-374547.html
Resurgamz
force-pushed
the
agent/pms-bug-bot/05ace414d55b
branch
from
September 24, 2026 08:36
e5f17ee to
875dac1
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰，无需修改。建议确认 Utils::getKeyshortcut 函数对所有可能的 QKeyEvent 均能返回有效字符串，避免 handleKeyPressEvent 接收到空字符串。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 1.建议将注释掉的快捷键注册代码直接删除，依赖版本控制历史保留记录，避免死代码积累 2.建议为 keyPressEvent 方法添加简要注释说明为何需要重载此方法（如'通过 Qt 事件系统直接处理按键，绕过 QAction 快捷键拦截'） 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理，无需优化。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 无安全风险。本次变更仅涉及键盘事件处理逻辑调整，不涉及用户输入校验、文件操作、网络通信或权限控制等安全敏感场景。 💡 改进建议代码示例// 建议删除注释掉的代码，而非保留
// Central.cpp 中直接移除以下注释行：
// keyList.append(QKeySequence(Qt::Key_Left));
// keyList.append(QKeySequence(Qt::Key_Right));
// ...
// 建议为 keyPressEvent 添加注释
/**
* @brief 重载按键事件处理
* 通过 Qt 事件系统直接处理按键，绕过 QAction 快捷键拦截
* @param event 按键事件
*/
void SlideWidget::keyPressEvent(QKeyEvent *event)
{
QString key = Utils::getKeyshortcut(event);
handleKeyPressEvent(key);
DWidget::keyPressEvent(event);
}本报告由 AI 代码审查工具自动生成 |
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
2 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.
Root Cause Analysis
Arrow keys (Left/Right/Up/Down) and Space were registered as QAction shortcuts in
Central.cpp's constructor. Qt's QAction shortcut mechanism intercepts these key events before they reachSheetBrowser'skeyPressEvent()default scroll handling. Meanwhile, the page navigation logic for left/right keys inCentralDocPage.cpp:664-667was already commented out, so the QAction trigger had no effect — it only blocked the default scrolling behavior.This is a regression: commit
a81e5e19previously fixed this exact issue (PMS 134413) by commenting out the QAction registrations, but merge commitb20fcf92(v23 code merge) reverted the fix.Fix
Comment out the QAction shortcut registrations for
Qt::Key_Left,Qt::Key_Right,Qt::Key_Up,Qt::Key_Down, andQt::Key_SpaceinCentral.cpp. Remove them_slideWidget->handleKeyPressEvent(s)forwarding call inCentralDocPage.cppsince keys no longer route through QAction. Add akeyPressEvent()override andsetFocus()inSlideWidgetconstructor so slide show mode handles arrow keys and space independently via its ownhandleKeyPressEvent().This restores the fix from commit
a81e5e19that was reverted by the v23 merge.Change Safety Assessment
Code Safety
keyPressEventoverride toSlideWidget— no function signatures changed, no external callers affected.a81e5e19applied the identical fix successfully; this change re-applies it after it was reverted by mergeb20fcf92.Business Impact Scope
SlideWidget::keyPressEvent()independently — no behavior change.Verification Suggestion
根因分析
Central.cpp构造函数中将Qt::Key_Left/Key_Right/Key_Up/Key_Down/Key_Space注册为 QAction 快捷键，Qt 快捷键机制在事件到达SheetBrowser的keyPressEvent()默认滚动处理之前拦截了这些按键。同时CentralDocPage.cpp:664-667中的左右键切页逻辑已被注释，QAction 触发后无任何动作，仅阻止了默认滚动行为。此为回归问题：commit
a81e5e19曾修复过相同问题（PMS 134413），合并 commitb20fcf92（v23 代码合并）回退了该修复。修复方案
注释
Central.cpp中 5 个方向键/空格的 QAction 快捷键注册；移除CentralDocPage.cpp中m_slideWidget->handleKeyPressEvent(s)转发调用；在SlideWidget构造函数中添加setFocus()，新增keyPressEvent()覆写，使幻灯片模式通过自身接收键盘事件。恢复历史 commit
a81e5e19的修复方案（被 v23 合并 commitb20fcf92回退）。改动安全评估
代码安全
SlideWidget新增keyPressEvent覆写，不涉及函数签名变更，无外部调用者受影响。a81e5e19曾以相同方式修复成功，本次为重新应用被回退的修复。业务影响范围
SlideWidget::keyPressEvent()自行处理，功能不变。验证建议
Summary by Sourcery
Restore normal reading keyboard scrolling while preserving independent keyboard controls for slide-show mode.
Bug Fixes:
Enhancements:
Tests: