add audio toolbar and AudioVolumeSlider widget - #511
Merged
Conversation
Reviewer's GuideIntroduces an AudioVolumeSlider widget that synchronizes music/sound volume with the global audio configuration, integrates it into a new main-window Audio toolbar and the audio preferences page, adds tests for the slider/config interaction, and makes several small refactors/cleanups in media and UI classes. Sequence diagram for AudioVolumeSlider updating configuration on user changesequenceDiagram
actor User
participant AudioVolumeSlider
participant QSlider
participant Configuration
participant AudioSettings
User->>AudioVolumeSlider: drag slider to new position
AudioVolumeSlider->>QSlider: setValue(newValue)
QSlider-->>AudioVolumeSlider: valueChanged(newValue)
AudioVolumeSlider->>AudioVolumeSlider: updateToConfig(newValue)
AudioVolumeSlider->>Configuration: setConfig()
Configuration-->>AudioVolumeSlider: audio settings reference
AudioVolumeSlider->>AudioSettings: getMusicVolume()/getSoundVolume()
AudioSettings-->>AudioVolumeSlider: currentVolume
AudioVolumeSlider->>AudioSettings: setMusicVolume(newValue) or setSoundVolume(newValue)
AudioVolumeSlider->>AudioSettings: setUnlocked()
Sequence diagram for configuration changes propagating back to AudioVolumeSlidersequenceDiagram
participant Configuration
participant AudioSettings
participant AudioVolumeSlider
participant QSlider
AudioVolumeSlider->>Configuration: setConfig().audio.registerChangeCallback(m_lifetime, callback)
Configuration-->>AudioVolumeSlider: store callback with lifetime
AudioSettings->>Configuration: internal volume change
Configuration->>AudioSettings: notify registered callbacks
AudioSettings-->>AudioVolumeSlider: invoke callback()
AudioVolumeSlider->>AudioVolumeSlider: updateFromConfig()
AudioVolumeSlider->>Configuration: getConfig()
Configuration-->>AudioVolumeSlider: const audio settings
AudioVolumeSlider->>AudioSettings: getMusicVolume()/getSoundVolume()
AudioSettings-->>AudioVolumeSlider: actualVolume
AudioVolumeSlider->>QSlider: setValue(actualVolume) (with SignalBlocker)
QSlider-->>AudioVolumeSlider: valueChanged(actualVolume)
AudioVolumeSlider->>AudioVolumeSlider: ignored due to SignalBlocker
Class diagram for the new AudioVolumeSlider and its integrationclassDiagram
direction LR
class AudioVolumeSlider {
<<QSlider>>
+enum AudioType
-Signal2Lifetime m_lifetime
-AudioType m_type
+AudioVolumeSlider(QWidget *parent)
+AudioVolumeSlider(AudioType type, QWidget *parent)
+~AudioVolumeSlider()
+AudioType audioType() const
+void setAudioType(AudioType type)
+void updateFromConfig()
+void wheelEvent(QWheelEvent *event)
-void init()
-void updateToConfig(int value)
}
class AudioPage {
<<QWidget>>
+void slot_loadConfig()
+void slot_outputDeviceChanged(int index)
+void slot_updateDevices()
}
class MainWindow {
<<QMainWindow>>
-QToolBar *audioToolBar
+void setupToolBars()
+void setupMenuBar()
}
class AudioSettings {
<<ConfigSection>>
+int getMusicVolume() const
+int getSoundVolume() const
+void setMusicVolume(int value)
+void setSoundVolume(int value)
+void setUnlocked()
+void registerChangeCallback(Signal2Lifetime &lifetime, std::function<void()> callback)
}
class Configuration {
+static const Configuration &getConfig()
+static Configuration &setConfig()
+AudioSettings audio
}
class SignalBlocker {
+SignalBlocker(QSlider &slider)
}
class QToolBar
class QSlider
class QWidget
class QMainWindow
AudioVolumeSlider --|> QSlider
AudioPage --|> QWidget
MainWindow --|> QMainWindow
Configuration o-- AudioSettings : has
AudioVolumeSlider --> Configuration : uses getConfig
AudioVolumeSlider --> Configuration : uses setConfig
AudioVolumeSlider --> AudioSettings : reads_writes
AudioVolumeSlider --> SignalBlocker : uses
AudioPage --> AudioVolumeSlider : ui_members
AudioPage --> Configuration : uses getConfig
MainWindow --> QToolBar : creates
MainWindow --> AudioVolumeSlider : embeds_in_audioToolBar
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}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In AudioVolumeSlider::setAudioType you use
toolTip().length() > 0as an implicit initialization guard; consider replacing this with an explicit boolean flag or a clearer state check to avoid relying on UI text as program logic. - Both AudioVolumeSlider and MapZoomSlider override wheelEvent to always
ignore()the event; if the intent is to fully disable wheel-based value changes, you may want to document this behavior or consider consuming the event instead to avoid confusing focus/scroll interactions in complex layouts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In AudioVolumeSlider::setAudioType you use `toolTip().length() > 0` as an implicit initialization guard; consider replacing this with an explicit boolean flag or a clearer state check to avoid relying on UI text as program logic.
- Both AudioVolumeSlider and MapZoomSlider override wheelEvent to always `ignore()` the event; if the intent is to fully disable wheel-based value changes, you may want to document this behavior or consider consuming the event instead to avoid confusing focus/scroll interactions in complex layouts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #511 +/- ##
==========================================
+ Coverage 25.04% 25.21% +0.16%
==========================================
Files 510 512 +2
Lines 42292 42367 +75
Branches 4577 4577
==========================================
+ Hits 10594 10681 +87
+ Misses 31698 31686 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Summary by Sourcery
Introduce a reusable AudioVolumeSlider widget and integrate an audio toolbar into the main window while tightening audio-related ownership, configuration wiring, and slider behavior.
New Features:
Enhancements:
Build:
Tests: