feat: add wheel event support for brightness and volume controls#340
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich 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 GuideIntroduces mouse wheel event handling in the brightness and sound applets by installing event filters on slider widgets, processing QWheelEvent in eventFilter overrides, and adjusting values in 2% increments. Sequence diagram for handling mouse wheel event on brightness slidersequenceDiagram
actor User
participant BrightnessSlider as Brightness Slider
participant BrightnessApplet
participant BrightnessController
User->>BrightnessSlider: Scroll mouse wheel
BrightnessSlider->>BrightnessApplet: eventFilter(QWheelEvent)
BrightnessApplet->>BrightnessSlider: updateSliderValue(newValue)
BrightnessApplet->>BrightnessController: setMonitorBrightness(newValue)
Sequence diagram for handling mouse wheel event on volume slidersequenceDiagram
actor User
participant VolumeSlider as Volume Slider
participant SoundApplet
participant SoundController
User->>VolumeSlider: Scroll mouse wheel
VolumeSlider->>SoundApplet: eventFilter(QWheelEvent)
SoundApplet->>VolumeSlider: setValue(newValue)
SoundApplet->>SoundController: SetVolume(newValue)
Class diagram for updated event filter handling in BrightnessApplet and SoundAppletclassDiagram
class BrightnessApplet {
+bool eventFilter(QObject *watcher, QEvent *event) override
}
class SoundApplet {
+bool eventFilter(QObject *watcher, QEvent *event) override
}
BrightnessApplet --|> QWidget
SoundApplet --|> QWidget
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @wjyrich - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `plugins/dde-dock/sound/soundapplet.cpp:363` </location>
<code_context>
+ int currentValue = m_volumeSlider->value();
+ int newValue = qBound(m_volumeSlider->minimum(), currentValue + delta, m_volumeSlider->maximum());
+ m_volumeSlider->setValue(newValue);
+ SoundController::ref().SetVolume(newValue * 0.01, true);
+
+ event->accept();
</code_context>
<issue_to_address>
Multiplying by 0.01 assumes slider range is 0-100.
If the slider range changes, this calculation may produce incorrect results. Use the slider's min and max values for scaling instead.
</issue_to_address>
### Comment 2
<location> `plugins/dde-dock/sound/soundapplet.cpp:92` </location>
<code_context>
m_listView->installEventFilter(this);
+
+ // 安装事件过滤器,以便处理滚轮事件
+ m_sliderContainer->installEventFilter(this);
+ m_volumeSlider->slider()->installEventFilter(this);
// sound setting button
</code_context>
<issue_to_address>
Installing event filters on both container and slider may cause duplicate event handling.
Check that wheel events are not processed twice due to filters on both the container and the slider, as this could cause unintended side effects.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| SoundController::ref().SetVolume(newValue * 0.01, true); | ||
|
|
||
| event->accept(); | ||
| return true; |
There was a problem hiding this comment.
issue (bug_risk): Multiplying by 0.01 assumes slider range is 0-100.
If the slider range changes, this calculation may produce incorrect results. Use the slider's min and max values for scaling instead.
| // sound setting button | ||
| m_settingButton->setAutoShowPage(true); |
There was a problem hiding this comment.
issue (bug_risk): Installing event filters on both container and slider may cause duplicate event handling.
Check that wheel events are not processed twice due to filters on both the container and the slider, as this could cause unintended side effects.
1. Added wheel event handling for brightness slider in BrightnessApplet 2. Implemented wheel event handling for volume control in SoundApplet 3. Installed event filters on relevant slider widgets to capture wheel events 4. Adjusted brightness/volume values in steps of 2% per wheel tick 5. Added necessary QWheelEvent includes The changes allow users to adjust brightness and volume levels using mouse wheel directly on the dock applets, providing a more intuitive and convenient control method. This matches common UX patterns found in other desktop environments. feat: 为亮度和音量控制添加滚轮事件支持 1. 在 BrightnessApplet 中添加亮度滑块的滚轮事件处理 2. 在 SoundApplet 中实现音量控制的滚轮事件处理 3. 在相关滑块部件上安装事件过滤器以捕获滚轮事件 4. 每次滚轮滚动调整亮度/音量2% 5. 添加必要的 QWheelEvent 头文件 这些修改允许用户直接在dock应用上使用鼠标滚轮调整亮度和音量级别,提供了更 直观和方便的控制方式。这与其他桌面环境中常见的用户体验模式相匹配。 Pms: BUG-317615
deepin pr auto review代码审查意见:
综上所述,建议在代码中添加适当的注释,检查并修复潜在的错误,以及确保动态分配的内存被正确释放。 |
|
/forcemerge |
The changes allow users to adjust brightness and volume levels using mouse wheel directly on the dock applets, providing a more intuitive and convenient control method. This matches common UX patterns found in other desktop environments.
feat: 为亮度和音量控制添加滚轮事件支持
这些修改允许用户直接在dock应用上使用鼠标滚轮调整亮度和音量级别,提供了更
直观和方便的控制方式。这与其他桌面环境中常见的用户体验模式相匹配。
Pms: BUG-317615
Summary by Sourcery
Add mouse wheel support to brightness and volume sliders in dock applets to allow 2% incremental adjustments per wheel tick.
New Features:
Enhancements: