-
Notifications
You must be signed in to change notification settings - Fork 55
fix: add missing winIconRole to DockCombineModel #1128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
这个问题可以修正未来 dataModel 切换到 DockItemModel 后,无应用图标的 应用不会将窗口图标作为 fallback 图标的问题. Log:
deepin pr auto review代码审查意见:
综合以上意见,建议对代码进行如下修改: // 在DockCombineModel类中
static const int WinIconRole = RoleCombineModel::roleNames().key(MODEL_WINICON);
DockCombineModel::DockCombineModel(QAbstractItemModel *major, QAbstractItemModel *minor)
: QAbstractItemModel(major, minor)
{
m_roles = {
{TaskManager::ActionsRole, RoleCombineModel::roleNames().key(MODEL_ACTIONS)},
{TaskManager::NameRole, RoleCombineModel::roleNames().key(MODEL_NAME)},
{TaskManager::WinIdRole, RoleCombineModel::roleNames().key(MODEL_WINID)},
{TaskManager::WinIconRole, WinIconRole},
{TaskManager::WinTitleRole, RoleCombineModel::roleNames().key(MODEL_TITLE)}
};
}
// 在RoleCombineModel类中
QHash<int, QByteArray> RoleCombineModel::roleNames() const
{
static const QHash<int, QByteArray> roles = {
{TaskManager::ActionsRole, MODEL_ACTIONS},
{TaskManager::NameRole, MODEL_NAME},
{TaskManager::WinIdRole, MODEL_WINID},
{TaskManager::WinIconRole, MODEL_WINICON},
{TaskManager::WinTitleRole, MODEL_TITLE}
};
return roles;
}这样修改后,代码更加简洁、可读,并且避免了重复调用 |
Reviewer's GuideInclude the missing WinIconRole in DockCombineModel’s role mappings so that window icons can be used as fallback when application icons are unavailable. Sequence Diagram: Enabled Fallback Icon Retrieval via DockCombineModelsequenceDiagram
participant IconResolver as "Icon Resolution Logic"
participant DBM as DockCombineModel
IconResolver->>DBM: Request Application Icon (e.g., using AppIconRole)
DBM-->>IconResolver: Application Icon Data (or indicates unavailability)
alt Application Icon Unavailable
IconResolver->>DBM: Request Window Icon (using TaskManager::WinIconRole)
Note over DBM: Now correctly maps and handles TaskManager::WinIconRole due to PR changes.
DBM-->>IconResolver: Window Icon Data (fallback, if available from underlying source)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @BLumia - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
修正未来 dataModel 切换到 DockItemModel 后,无应用图标的应用不会将窗口图标作为 fallback 图标的问题.
(当前还未切换,不构成影响)
Summary by Sourcery
Add missing WinIconRole to DockCombineModel role mappings to ensure window icons can be used as fallback for applications without an icon.
Bug Fixes: