Skip to content

fix(exhibition): use emblem-checked icon for in-app notification - #737

Merged
18202781743 merged 1 commit into
linuxdeepin:v25-flowstylefrom
zqq-dora:fix-notification-icon-emblem-checked
Oct 28, 2026
Merged

18202781743 merged 1 commit into
linuxdeepin:v25-flowstylefrom
zqq-dora:fix-notification-icon-emblem-checked

Conversation

@zqq-dora

@zqq-dora zqq-dora commented Oct 28, 2026 •

Copy link
Copy Markdown

Background

The in-app notification example (examples/exhibition/Notify.qml) passed iconName="checked" when triggering the "成功添加到校园名谣" bubble message. This icon name does not exist in the library's built-in DCI icon set (built-in-icons), nor does a corresponding checked.dci exist in any system DCI icon theme (/usr/share/dsg/icons/<theme>/). After DCI resolution fails, DciIcon falls back via fallbackToQIcon to QIcon::fromTheme("checked"), but checked is not a freedesktop standard icon name, so the standard icon theme cannot resolve it either, ultimately rendering an incorrect icon.

Changes

examples/exhibition/Notify.qml

  • Changed the icon name in DTK.sendMessage(control, "成功添加到"校园名谣"", "checked") from "checked" to "emblem-checked".

emblem-checked is a standard name in the freedesktop icon naming specification, and emblem-checked.dci exists in all system DCI icon themes (flow, bloom, bloom-dark, bloom-classic, vintage, square, nirvana, hazy-color, organic-glass, origin). The resolution chain is: first search for emblem-checked.dci in the DCI theme, and render it according to the current theme if found; if the DCI theme is not installed, fallbackToQIcon (enabled by default) falls back to QIcon::fromTheme("emblem-checked"), resolving from the emblems/24/ directory of QIcon icon themes (e.g., bloom, bloom-dark). This icon is theme-independent and does not rely on library built-in icon resources.

Effect

  • The success checkmark icon in the in-app notification bubble is correctly rendered as the emblem-checked icon corresponding to the current theme.
  • Works correctly with the default origin theme as well as any other installed DCI icon theme.
  • Even if the system DCI icon theme package is not installed, the icon can still be resolved through the QIcon fallback layer.

Only example code (examples/exhibition/) is modified; Qt5/Qt6 control implementations are not affected.


背景

应用内通知示例(examples/exhibition/Notify.qml)在触发「成功添加到校园名谣」气泡消息时,传入 iconName="checked"。该图标名在库的内置 DCI 图标集(built-in-icons)中不存在,在系统 DCI 图标主题(/usr/share/dsg/icons/<theme>/)中也不存在对应的 checked.dci。DciIcon 解析失败后,通过 fallbackToQIcon 兜底到 QIcon::fromTheme("checked"),而 checked 并非 freedesktop 标准图标名,标准图标主题同样无法解析,最终渲染出错误的图标。

改动

examples/exhibition/Notify.qml

  • 将 DTK.sendMessage(control, "成功添加到"校园名谣"", "checked") 中的图标名从 "checked" 改为 "emblem-checked"。

emblem-checked 是 freedesktop 图标命名规范中的标准名称,同时存在于全部系统 DCI 图标主题(flow、bloom、bloom-dark、bloom-classic、vintage、square、nirvana、hazy-color、organic-glass、origin)的 emblem-checked.dci 中。解析链路为:先在 DCI 主题中查找 emblem-checked.dci,命中则按当前主题渲染;若 DCI 主题未安装,fallbackToQIcon(默认开启)兜底到 QIcon::fromTheme("emblem-checked"),在 QIcon 图标主题(如 bloom、bloom-dark 等)的 emblems/24/ 目录中解析。该图标与主题无关,不依赖库内置图标资源。

效果

  • 应用内通知气泡中的「成功」对钩图标正确渲染为当前主题对应的 emblem-checked 图标。
  • 无论是默认 origin 主题还是其他已安装的 DCI 图标主题,均能正确显示。
  • 即使系统未安装 DCI 图标主题包,仍可通过 QIcon 兜底层解析 emblem-checked。

仅修改示例代码(examples/exhibition/),不涉及 Qt5/Qt6 控件实现。

Summary by Sourcery

Correct the exhibition notification’s success icon by replacing the unresolved checked name with the standard emblem-checked icon.

Bug Fixes:

  • Use the standard emblem-checked icon name so the exhibition notification displays the correct success icon across available themes and fallback icon resolution.

Chores:

  • Update the Notify.qml copyright year range.

@sourcery-ai

sourcery-ai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The example notification now uses emblem-checked, a theme-supported standard icon name, so the success bubble resolves correctly through the DCI theme and can fall back to the QIcon theme without changing Qt5/Qt6 control implementations.

Sequence diagram for theme-aware notification icon resolution

sequenceDiagram
    participant User
    participant NotifyQml
    participant DTK
    participant DciIcon
    participant DCITheme
    participant QIconTheme

    User->>NotifyQml: Click notification example
    NotifyQml->>DTK: sendMessage(control, message, emblem-checked)
    DTK->>DciIcon: Resolve emblem-checked
    alt DCI theme installed
        DciIcon->>DCITheme: Find emblem-checked.dci
        DCITheme-->>DciIcon: Theme-specific icon
    else DCI theme unavailable
        DciIcon->>QIconTheme: fromTheme(emblem-checked)
        QIconTheme-->>DciIcon: Standard theme icon
    end
    DciIcon-->>DTK: Render resolved icon
    DTK-->>User: Show success notification
Loading

File-Level Changes

Change Details Files
Replace the unresolved notification icon name with the freedesktop-standard emblem-checked identifier.
  • Update the success notification example to request emblem-checked instead of checked.
  • Allow DCI theme lookup to resolve the icon first, with QIcon::fromTheme fallback remaining available when DCI assets are absent.
examples/exhibition/Notify.qml

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

@deepin-ci-robot

Copy link
Copy Markdown
Contributor

Hi @zqq-dora. Thanks for your PR.

I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@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 - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

The in-app notification example passed iconName="checked" which does
not exist in the built-in DCI icon set or any system DCI icon theme.
"checked" is also not a freedesktop standard icon name, so the QIcon
fallback cannot resolve it either, resulting in an incorrect icon.

Change to "emblem-checked", a freedesktop standard icon name that
exists in all system DCI icon themes. The resolution chain works as:
DCI theme (emblem-checked.dci) -> QIcon fallback (emblems/24/).

Also update the SPDX copyright year from 2022 to 2022-2026.
@zqq-dora
zqq-dora force-pushed the fix-notification-icon-emblem-checked branch from 39da8fc to e066444 Compare October 28, 2026 06:39
source: "qrc:/assets/popup/nointeractive.svg"
onClicked: {
DTK.sendMessage(control, "成功添加到"校园名谣"", "checked")
DTK.sendMessage(control, "成功添加到"校园名谣"", "emblem-checked")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

有这个图么?

@zqq-dora

Copy link
Copy Markdown
Author

有的。emblem-checked 是 freedesktop 图标命名规范中的标准名称,系统 DCI 图标主题(origin、bloom、bloom-dark、bloom-classic、bloom-classic-dark、vintage、square、nirvana、hazy-color、flow、organic-glass)均有 emblem-checked.dci,QIcon 兜底层(bloom、bloom-dark、vintage、hazy-color、flow)的 emblems/24/ 目录中也有对应资源。

@deepin-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, zqq-dora

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

@18202781743
18202781743 merged commit f12c5d3 into linuxdeepin:v25-flowstyle Oct 28, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants