fix: sound plugin display incorrect volume tips by yixinshark · Pull Request #315 · linuxdeepin/dde-tray-loader

yixinshark · 2025-05-20T08:50:43Z

mute status changed, then update volume tips, false status revert to zero, so tips show 0 value

Log: as title
Pms: BUG-313463

Summary by Sourcery

Simplify the SoundView refresh logic by removing explicit volume arguments and always fetching the latest volume inside refreshTips, resolving wrong volume displays after mute toggles.

Bug Fixes:

Fix incorrect volume tip display by ensuring volume is fetched after mute status changes

Enhancements:

Refactor SoundView by removing explicit volume parameters and centralizing volume retrieval in refreshTips

sourcery-ai · 2025-05-20T08:53:07Z

Reviewer's Guide

This PR refactors SoundView so that volume is no longer passed around but retrieved internally when updating the UI, ensuring the displayed volume tip reflects the current mute and volume state correctly.

File-Level Changes

Change	Details	Files
Refactored refresh and refreshTips methods to remove external volume parameters	Changed SoundView::refresh signature from taking an int to a parameterless method Changed SoundView::refreshTips signature to accept only the force flag Updated all calls (constructor, tipsWidget, eventFilter) to use the new method signatures	`plugins/dde-dock/sound/soundview.cpp` `plugins/dde-dock/sound/soundview.h`
Moved volume retrieval into refreshTips for consistent tip updates	Removed external volume argument and call to SoundModel::ref().volume() in callers Introduced local auto volume = std::min(150, SoundModel::ref().volume()) inside refreshTips Adjusted label text formatting to use the internally computed volume	`plugins/dde-dock/sound/soundview.cpp`

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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @yixinshark - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 2 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

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

sourcery-ai · 2025-05-20T08:56:04Z

            m_tipsLabel->setText(QString(tr("Mute")));
        } else {
+            auto volume = std::min(150, SoundModel::ref().volume());
            m_tipsLabel->setText(QString(tr("Volume %1").arg(QString::number(volume) + '%')));


suggestion: Include the percent sign in the translatable string and simplify arg usage

Use tr("Volume %1% ").arg(volume) to include '%' in the translatable string and remove manual concatenation and extra QString wrappers.

Suggested change

m_tipsLabel->setText(QString(tr("Volume %1").arg(QString::number(volume) + '%')));

m_tipsLabel->setText(tr("Volume %1%").arg(volume));

sourcery-ai · 2025-05-20T08:56:04Z

        if (SoundModel::ref().isMute()) {
            m_tipsLabel->setText(QString(tr("Mute")));
        } else {
+            auto volume = std::min(150, SoundModel::ref().volume());


suggestion: Avoid magic number for max volume clamp

Define a named constant (e.g. MAX_VOLUME) or move the clamp into SoundModel so the view doesn’t use a hard-coded limit.

mute status changed, then update volume tips, false status revert to zero, so tips show 0 value Log: as title Pms: BUG-313463

deepin-ci-robot · 2025-05-20T09:39:37Z

deepin pr auto review

代码审查意见：

在soundcontroller.cpp文件中，SoundModel::ref().setVolume(m_defaultSinkInter->volume());这行代码被移动到了MuteChanged信号处理函数中，但VolumeChanged信号处理函数中仍然保留了SoundModel::ref().setVolume(value);。建议统一处理，如果MuteChanged信号处理函数已经更新了音量，则不需要在VolumeChanged信号处理函数中再次更新音量。
在soundview.cpp文件中，SoundView::SoundView(QWidget *parent)构造函数中，refresh(SoundModel::ref().volume());被替换为refresh();。这可能会导致在构造函数执行时，音量状态没有正确刷新。建议在构造函数中保留对音量的刷新，或者确保在构造函数执行后立即调用refresh()。
在soundview.cpp文件中，SoundView::refreshTips(const int volume, const bool force)函数被重载为SoundView::refreshTips(const bool force)。这可能会导致在调用refreshTips时，无法正确传递音量信息。建议保留refreshTips的重载版本，并在需要时传递音量信息。
在soundview.cpp文件中，SoundView::refreshTips函数中，auto volume = std::min(150, SoundModel::ref().volume());这行代码被移动到了refreshTips函数内部。这可能会导致在refreshTips函数被多次调用时，每次都会重新获取音量信息。建议将音量信息作为参数传递给refreshTips函数，或者将其存储在类的成员变量中，以避免重复获取。
在soundview.cpp文件中，SoundView::refreshTips函数中，m_tipsLabel->setText(QString(tr("Volume %1").arg(QString::number(volume) + '%')));这行代码使用了QString::number(volume)来格式化音量，但没有检查音量是否超出范围。建议添加对音量的范围检查，以确保音量值在有效范围内。
在soundview.h文件中，SoundView::refreshTips函数的声明被更新为void refreshTips(const bool force = false);，但SoundView::refreshTips函数的实现中仍然使用了const int volume参数。建议更新函数实现以匹配函数声明。

以上是针对代码审查的几点意见，希望能对您有所帮助。

deepin-ci-robot · 2025-05-20T11:02:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sourcery-ai Bot reviewed May 20, 2025

View reviewed changes

yixinshark force-pushed the fix-0Volume branch from bfd79e8 to ba8c35b Compare May 20, 2025 09:36

fix: sound plugin display incorrect volume tips

203b1a0

mute status changed, then update volume tips, false status revert to zero, so tips show 0 value Log: as title Pms: BUG-313463

yixinshark force-pushed the fix-0Volume branch from ba8c35b to 203b1a0 Compare May 20, 2025 09:39

18202781743 approved these changes May 20, 2025

View reviewed changes

yixinshark merged commit 14a8cfe into linuxdeepin:master May 21, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: sound plugin display incorrect volume tips#315

fix: sound plugin display incorrect volume tips#315
yixinshark merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix-0Volume

yixinshark commented May 20, 2025 •

edited by sourcery-ai Bot

Loading

Uh oh!

sourcery-ai Bot commented May 20, 2025

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

sourcery-ai Bot May 20, 2025

Uh oh!

sourcery-ai Bot May 20, 2025

Uh oh!

deepin-ci-robot commented May 20, 2025

Uh oh!

deepin-ci-robot commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	m_tipsLabel->setText(QString(tr("Volume %1").arg(QString::number(volume) + '%')));
	m_tipsLabel->setText(tr("Volume %1%").arg(volume));

Conversation

yixinshark commented May 20, 2025 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai Bot commented May 20, 2025

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot May 20, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot May 20, 2025

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented May 20, 2025

deepin pr auto review

Uh oh!

deepin-ci-robot commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yixinshark commented May 20, 2025 •

edited by sourcery-ai Bot

Loading