Skip to content

feat: improve battery icon loading strategy#348

Merged
18202781743 merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-323767
Aug 7, 2025
Merged

feat: improve battery icon loading strategy#348
18202781743 merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-323767

Conversation

@wjyrich

@wjyrich wjyrich commented Aug 7, 2025

Copy link
Copy Markdown
Contributor
  1. Modified battery icon loading to first try system theme icons before falling back to built-in resources
  2. Added proper handling for dark theme variants by appending "-dark" suffix
  3. Improved SVG renderer fallback logic when theme icons are not available
  4. Fixed resource path inconsistency in PowerStatusWidget
  5. Added proper error handling for invalid SVG files

The changes improve icon loading reliability and maintainability by:

  • Supporting system theme icons which allows for better customization
  • Providing proper fallback mechanisms when theme icons are missing
  • Ensuring consistent behavior across light/dark themes
  • Fixing resource path inconsistencies between components

feat: 改进电池图标加载策略

  1. 修改电池图标加载逻辑,优先尝试系统主题图标,再回退到内置资源
  2. 添加了对暗色主题变体的正确处理,通过添加"-dark"后缀
  3. 改进了当主题图标不可用时的SVG渲染器回退逻辑
  4. 修复了PowerStatusWidget中的资源路径不一致问题
  5. 为无效SVG文件添加了适当的错误处理

这些改进通过以下方式提高了图标加载的可靠性和可维护性:

  • 支持系统主题图标,允许更好的自定义
  • 当主题图标缺失时提供适当的回退机制
  • 确保在亮/暗主题下行为一致
  • 修复组件间的资源路径不一致问题 Pms: BUG-323767

Summary by Sourcery

Improve battery icon loading by leveraging system theme icons, supporting dark theme variants, and enhancing fallback logic while fixing resource path inconsistencies.

New Features:

  • Load battery icons from the system theme before falling back to built-in resources

Bug Fixes:

  • Fix resource path inconsistency in PowerStatusWidget icon loading

Enhancements:

  • Append "-dark" suffix for dark theme variants to ensure correct icon selection
  • Enhance SVG renderer fallback by validating the renderer and loading a default resource if invalid
  • Add error handling for invalid SVG files during icon rendering

@sourcery-ai

sourcery-ai Bot commented Aug 7, 2025

Copy link
Copy Markdown

Reviewer's Guide

Implements a multi-stage battery icon loader that first attempts system theme icons (including dark variants), then falls back to built-in SVG resources with robust error handling, and fixes a resource path inconsistency in the PowerStatusWidget.

Sequence diagram for improved battery icon loading strategy

sequenceDiagram
    participant PowerStatusWidget
    participant PowerApplet
    participant QIcon
    participant QSvgRenderer
    participant m_batteryIcon
    PowerStatusWidget->>PowerApplet: refreshBatteryIcon(iconStr)
    PowerApplet->>QIcon: fromTheme(iconName)
    alt Theme icon exists
        QIcon-->>PowerApplet: themeIcon
        PowerApplet->>m_batteryIcon: setPixmap(themeIcon.pixmap())
    else Theme icon missing
        PowerApplet->>QSvgRenderer: QSvgRenderer(fallbackPath)
        alt fallbackPath invalid
            PowerApplet->>QSvgRenderer: load(normalPath)
        end
        PowerApplet->>m_batteryIcon: setPixmap(rendered SVG)
    end
Loading

Class diagram for updated PowerApplet and PowerStatusWidget icon logic

classDiagram
    class PowerStatusWidget {
        +void refreshIcon()
        -m_iconWidget
        -m_applet
    }
    class PowerApplet {
        +void refreshBatteryIcon(QString icon)
        -m_batteryIcon
    }
    class QIcon {
        +static QIcon fromTheme(QString iconName)
        +QPixmap pixmap(QSize size)
    }
    class QSvgRenderer {
        +QSvgRenderer(QString path)
        +bool isValid()
        +void load(QString path)
    }
    PowerStatusWidget --> PowerApplet : uses
    PowerApplet --> QIcon : uses
    PowerApplet --> QSvgRenderer : uses
Loading

File-Level Changes

Change Details Files
Enhanced battery icon loading with theme support and robust fallback
  • Try loading icons from the system theme via QIcon::fromTheme before using built-in resources
  • Append “-dark” suffix for dark-theme variants when appropriate
  • Use QSvgRenderer for fallback SVGs and load a secondary path if the renderer is invalid
  • Return early on successful theme icon load to avoid unnecessary SVG rendering
plugins/dde-dock/power/powerapplet.cpp
Fixed resource path inconsistency in PowerStatusWidget
  • Corrected the icon resource path passed to m_iconWidget->setIcon to match the built-in directory structure
plugins/dde-dock/power/powerstatuswidget.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

1. Modified battery icon loading to first try system theme icons before
falling back to built-in resources
2. Added proper handling for dark theme variants by appending "-dark"
suffix
3. Improved SVG renderer fallback logic when theme icons are not
available
4. Fixed resource path inconsistency in PowerStatusWidget
5. Added proper error handling for invalid SVG files

The changes improve icon loading reliability and maintainability by:
- Supporting system theme icons which allows for better customization
- Providing proper fallback mechanisms when theme icons are missing
- Ensuring consistent behavior across light/dark themes
- Fixing resource path inconsistencies between components

feat: 改进电池图标加载策略

1. 修改电池图标加载逻辑,优先尝试系统主题图标,再回退到内置资源
2. 添加了对暗色主题变体的正确处理,通过添加"-dark"后缀
3. 改进了当主题图标不可用时的SVG渲染器回退逻辑
4. 修复了PowerStatusWidget中的资源路径不一致问题
5. 为无效SVG文件添加了适当的错误处理

这些改进通过以下方式提高了图标加载的可靠性和可维护性:
- 支持系统主题图标,允许更好的自定义
- 当主题图标缺失时提供适当的回退机制
- 确保在亮/暗主题下行为一致
- 修复组件间的资源路径不一致问题
Pms: BUG-323767
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查意见:

  1. 代码重复

    • powerapplet.cpp文件中,fallbackPath的构建逻辑被重复了两次,建议提取成一个函数或变量,以减少代码重复。
  2. 图标路径硬编码

    • 图标路径被硬编码在代码中,建议使用配置文件或常量定义路径,以便于维护和修改。
  3. 资源文件加载失败处理

    • powerapplet.cpp文件中,当QSvgRenderer加载失败时,只是简单地加载了另一个路径,但没有处理加载失败的情况。建议添加错误处理逻辑,例如记录日志或显示默认图标。
  4. 图标大小和设备像素比

    • powerapplet.cpp文件中,QImage的创建使用了devicePixelRatioF(),但没有对QSvgRenderer进行相应的缩放处理。建议在加载SVG图标时也考虑设备像素比,以确保图标在不同分辨率下显示正确。
  5. 图标主题加载失败处理

    • powerapplet.cpp文件中,当QIcon::fromTheme加载失败时,只是简单地返回,没有进行任何错误处理或提示。建议添加错误处理逻辑,例如记录日志或显示默认图标。
  6. 路径拼接错误

    • powerstatuswidget.cpp文件中,路径拼接时使用了+/,这是不正确的。应该使用+进行字符串拼接。
  7. 图标路径硬编码

    • powerstatuswidget.cpp文件中,图标路径被硬编码在代码中,建议使用配置文件或常量定义路径,以便于维护和修改。
  8. 资源文件加载失败处理

    • powerstatuswidget.cpp文件中,没有对m_iconWidget->setIcon的调用结果进行检查,建议添加错误处理逻辑,例如记录日志或显示默认图标。

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

@18202781743 18202781743 merged commit b7d8fbf into linuxdeepin:master Aug 7, 2025
8 of 9 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