-
Notifications
You must be signed in to change notification settings - Fork 55
feat: implement window split mode with dynamic title display #1354
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
Reviewer's GuideImplements a window split mode for the dock task manager that shows each window as a separate taskbar item with dynamically sized titles, supports per-window drag-and-drop and actions, and updates the model and context menus to handle individual window titles and close behavior. Sequence diagram for context menu Close action in window split modesequenceDiagram
actor User
participant AppItemWithTitle
participant TaskManagerQml
participant DockGlobalElementModel
participant TaskManagerSettings
User->>AppItemWithTitle: RightClick()
AppItemWithTitle->>AppItemWithTitle: BuildContextMenuFromMenusJson()
User->>AppItemWithTitle: SelectCloseMenuItem()
AppItemWithTitle->>TaskManagerQml: TaskManager_requestNewInstance(modelIndex DOCK_ACTION_CLOSEWINDOW)
TaskManagerQml->>DockGlobalElementModel: requestNewInstance(index action)
DockGlobalElementModel->>DockGlobalElementModel: Read m_data at index
DockGlobalElementModel->>DockGlobalElementModel: Detect action DOCK_ACTION_CLOSEWINDOW
DockGlobalElementModel->>DockGlobalElementModel: requestClose(index false)
DockGlobalElementModel-->>TaskManagerQml: ReturnFromRequestNewInstance()
TaskManagerQml-->>AppItemWithTitle: ReturnFromRequestNewInstance()
AppItemWithTitle-->>User: WindowClosedOnTaskbarItem()
Note over DockGlobalElementModel,TaskManagerSettings: If not in window split mode, menu uses DOCK_ACTION_CLOSEALL instead, which also calls requestClose(index false) but targets all windows of the app
Sequence diagram for per-window drag and drop reordering in window split modesequenceDiagram
actor User
participant AppItemWithTitle
participant LauncherDndDropArea
participant TaskManagerQml
participant VisualModel
User->>AppItemWithTitle: PressAndStartDrag()
AppItemWithTitle->>LauncherDndDropArea: DragEnter(appId winId)
LauncherDndDropArea->>LauncherDndDropArea: Store launcherDndDesktopId
LauncherDndDropArea->>LauncherDndDropArea: Store launcherDndDragSource taskbar
LauncherDndDropArea->>LauncherDndDropArea: Store launcherDndWinId
User->>LauncherDndDropArea: DragMove(x y)
LauncherDndDropArea->>TaskManagerQml: onPositionChanged(x y)
TaskManagerQml->>TaskManagerQml: appId = desktopIdToAppId(launcherDndDesktopId)
TaskManagerQml->>TaskManagerQml: currentIndex = findAppIndexByWindow(appId launcherDndWinId)
TaskManagerQml->>VisualModel: targetIndex = indexAt(x y)
TaskManagerQml->>VisualModel: items.move(currentIndex targetIndex)
User->>LauncherDndDropArea: Drop(x y)
LauncherDndDropArea->>TaskManagerQml: onDropped(x y)
TaskManagerQml->>TaskManagerQml: appId = desktopIdToAppId(launcherDndDesktopId)
TaskManagerQml->>TaskManagerQml: currentIndex = findAppIndexByWindow(appId launcherDndWinId)
TaskManagerQml->>VisualModel: items.move(currentIndex targetIndex)
TaskManagerQml->>LauncherDndDropArea: resetDndState()
AppItemWithTitle->>TaskManagerQml: onDragFinished()
TaskManagerQml-->>User: WindowsOfSameAppRemainGrouped()
Class diagram for DockGlobalElementModel and AppItemWithTitle changesclassDiagram
class DockGlobalElementModel {
- QAbstractItemModel appsModel
- QAbstractItemModel activeAppModel
- QList~tuple~ m_data
+ DockGlobalElementModel(appsModel activeModel parent)
+ data(index role) QVariant
+ getMenus(index) QString
+ requestActivate(index) void
+ requestNewInstance(index action) void
+ requestOpenUrls(index urls) void
+ requestClose(index force) void
}
class TaskManagerSettings {
+ instance() TaskManagerSettings
+ isWindowSplit() bool
+ isAllowedForceQuit() bool
+ toggleDockedElement(desktopId) void
}
class ActiveAppModel {
+ index(row column) QModelIndex
+ requestNewInstance(index action) void
+ data(index role) QVariant
}
class AppsModel {
+ index(row column) QModelIndex
+ data(index role) QVariant
}
class AppItemWithTitle {
+ int displayMode
+ int colorTheme
+ bool active
+ bool attention
+ string itemId
+ string name
+ string windowTitle
+ string iconName
+ string menus
+ list~string~ windows
+ int visualIndex
+ var modelIndex
+ int maxCharLimit
+ int iconSize
+ int textSize
+ string displayText
+ int actualWidth
+ dropFilesOnItem(itemId files)
+ dragFinished()
}
class TaskManagerQml {
+ bool windowSplit
+ var dynamicCharLimits
+ int remainingSpacesForSplitWindow
+ findAppIndex(appId) int
+ findAppIndexByWindow(appId winId) int
+ calculateTextWidth(text textSize) int
+ calculateMaxCharsWithinWidth(title maxWidth textSize) int
+ calculateItemWidth(title maxChars iconSize textSize) int
+ calculateTotalWidth(charLimits iconSize textSize) int
+ findLongestTitleIndex(charLimits) int
+ calculateDynamicCharLimits(remainingSpace iconSize textSize) var
+ updateDynamicCharLimits() void
}
DockGlobalElementModel --> ActiveAppModel : uses
DockGlobalElementModel --> AppsModel : uses
DockGlobalElementModel --> TaskManagerSettings : uses
AppItemWithTitle --> TaskManagerQml : uses
AppItemWithTitle --> DockGlobalElementModel : calls requestNewInstance
Class diagram for drag and drop and visibility handling in TaskManager QML delegatesclassDiagram
class TaskManagerQmlDelegateRoot {
+ bool attention
+ string itemId
+ string name
+ string title
+ string iconName
+ string icon
+ string menus
+ list~string~ windows
+ bool visibility
+ int dynamicCharLimit
+ int implicitWidth
+ int implicitHeight
+ int visualIndex
+ var modelIndex
}
class LauncherDndDropArea {
+ string launcherDndDesktopId
+ string launcherDndDragSource
+ string launcherDndWinId
+ resetDndState() void
+ onEntered(drag) void
+ onPositionChanged(drag) void
+ onDropped(drop) void
}
class TaskManagerAppletQml {
+ bool windowSplit
+ desktopIdToAppId(desktopId) string
+ requestDockByDesktopId(desktopId) bool
+ requestUpdateWindowIconGeometry(modelIndex rect panelRoot) void
+ requestPreview(modelIndex panelRoot xOffset yOffset position) void
+ hideItemPreview() void
}
TaskManagerQmlDelegateRoot --> LauncherDndDropArea : uses drag state
TaskManagerQmlDelegateRoot --> TaskManagerAppletQml : calls preview and geometry
LauncherDndDropArea --> TaskManagerAppletQml : calls requestDockByDesktopId
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 there - I've reviewed your changes - here's some feedback:
- AppItemWithTitle.qml duplicates a large amount of logic from AppItem (status indicators, animations, menus, tooltips, drag/drop, etc.); consider extracting a shared base component or mixin for the common behavior so future changes don’t have to be maintained in two places.
- In DockGlobalElementModel::requestNewInstance you call QProcess::waitForFinished(), which can block the main thread; consider using startDetached or asynchronous process handling instead so the UI is not stalled while dde-am runs.
- remainingSpacesForSplitWindow and the dynamic title width calculations don't appear to guard against negative or very small values; it may be safer to clamp remainingSpacesForSplitWindow and/or early‑return when the available width is below a minimal threshold to avoid strange layout behavior under extreme dock configurations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- AppItemWithTitle.qml duplicates a large amount of logic from AppItem (status indicators, animations, menus, tooltips, drag/drop, etc.); consider extracting a shared base component or mixin for the common behavior so future changes don’t have to be maintained in two places.
- In DockGlobalElementModel::requestNewInstance you call QProcess::waitForFinished(), which can block the main thread; consider using startDetached or asynchronous process handling instead so the UI is not stalled while dde-am runs.
- remainingSpacesForSplitWindow and the dynamic title width calculations don't appear to guard against negative or very small values; it may be safer to clamp remainingSpacesForSplitWindow and/or early‑return when the available width is below a minimal threshold to avoid strange layout behavior under extreme dock configurations.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:409-418` </location>
<code_context>
+void DockGlobalElementModel::requestNewInstance(const QModelIndex &index, const QString &action) const
</code_context>
<issue_to_address>
**issue (performance):** Avoid blocking the UI thread by waiting synchronously on QProcess when dispatching app actions.
In requestNewInstance, QProcess is started and then waitForFinished() is called, which will block the calling (likely GUI) thread until dde-am exits and can cause visible UI freezes. Instead, consider starting the process without blocking (e.g., QProcess::startDetached or an asynchronous start) and handle errors via signals or logging.
</issue_to_address>
### Comment 2
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:369-376` </location>
<code_context>
return QStringList{model->index(row, 0).data(TaskManager::WinIdRole).toString()};
}
+ }
+ case TaskManager::WinTitleRole: {
+ // Only active windows have window titles
+ if (model == m_activeAppModel) {
+ return model->index(row, 0).data(TaskManager::WinTitleRole);
+ }
// For m_appsModel data, when it's GroupModel we can directly get all window IDs for this desktop ID
QModelIndex groupIndex = model->index(row, 0);
return groupIndex.data(TaskManager::WindowsRole).toStringList();
</code_context>
<issue_to_address>
**issue (bug_risk):** WinTitleRole handling falls through from WindowsRole and returns window IDs for non-active apps.
Because there’s no break after the WindowsRole case, non-active models fall through into WinTitleRole and return groupIndex.data(TaskManager::WindowsRole) (a list of window IDs) instead of a title. This doesn’t match the WinTitleRole semantics or the QML delegate’s expectation of a string. Please add an explicit break after WindowsRole, and have WinTitleRole return an empty QVariant or a suitable placeholder for non-active apps rather than reusing the WindowsRole data.
</issue_to_address>
### Comment 3
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:65` </location>
<code_context>
auto index = m_activeAppModel->index(i, 0);
auto desktopId = index.data(TaskManager::DesktopIdRole).toString();
- auto it = std::find_if(m_data.begin(), m_data.end(), [this, &desktopId](auto &data) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting small helper functions for window insertion, action handling, and window-related data retrieval to reduce duplication and make the control flow easier to follow.
You can simplify the new logic without changing behavior by extracting a couple of focused helpers and removing some duplication.
1) Window insertion/grouping logic in the rowsInserted lambda
Right now the lambda does:
- early `continue`s,
- `find_if` + manual loop for `lastIt`,
- repeated `std::make_tuple(desktopId, m_activeAppModel, i)`.
This can be moved into a small helper that:
- computes the insert/update position,
- performs the insert/update,
- emits the signals.
Example refactor outline:
```cpp
// in DockGlobalElementModel (private)
int DockGlobalElementModel::findInsertPositionForDesktop(const QString &desktopId) const
{
auto firstIt = std::find_if(m_data.begin(), m_data.end(),
[&desktopId](const auto &data) {
return std::get<0>(data) == desktopId;
});
if (firstIt == m_data.end()) {
// no existing item -> append
return m_data.size();
}
if (std::get<1>(*firstIt) == m_appsModel) {
// reuse docked position
return int(firstIt - m_data.begin());
}
// insert after last window for this desktop
auto lastIt = firstIt;
while (lastIt + 1 != m_data.end()
&& std::get<0>(*(lastIt + 1)) == desktopId) {
++lastIt;
}
return int(lastIt - m_data.begin()) + 1;
}
void DockGlobalElementModel::insertOrUpdateActiveWindow(const QString &desktopId, int sourceRow)
{
const auto idx = findInsertPositionForDesktop(desktopId);
auto tuple = std::make_tuple(desktopId, m_activeAppModel, sourceRow);
if (idx == m_data.size()) {
// append
beginInsertRows(QModelIndex(), idx, idx);
m_data.insert(m_data.begin() + idx, tuple);
endInsertRows();
} else {
// update or insert in the middle
auto it = m_data.begin() + idx;
if (std::get<1>(*it) == m_appsModel) {
*it = tuple;
auto pIndex = index(idx, 0);
Q_EMIT dataChanged(pIndex,
pIndex,
{TaskManager::ActiveRole,
TaskManager::AttentionRole,
TaskManager::WindowsRole,
TaskManager::MenusRole,
TaskManager::WinTitleRole});
} else {
beginInsertRows(QModelIndex(), idx, idx);
m_data.insert(it, tuple);
endInsertRows();
}
}
}
```
Then the lambda becomes much easier to follow:
```cpp
connect(m_activeAppModel, &QAbstractItemModel::rowsInserted, this,
[this](const QModelIndex &parent, int first, int last) {
Q_UNUSED(parent)
for (int i = first; i <= last; ++i) {
auto index = m_activeAppModel->index(i, 0);
auto desktopId = index.data(TaskManager::DesktopIdRole).toString();
if (desktopId.isEmpty())
continue;
insertOrUpdateActiveWindow(desktopId, i);
}
std::for_each(m_data.begin(), m_data.end(),
[this, first, last](auto &data) {
if (std::get<1>(data) == m_activeAppModel
&& std::get<2>(data) > first) {
data = std::make_tuple(
std::get<0>(data),
std::get<1>(data),
std::get<2>(data) + ((last - first) + 1));
}
});
},
Qt::QueuedConnection);
```
This keeps all behavior (grouping, reuse of docked position, signals, WinTitleRole) but removes local iterator arithmetic and tuple repetition from the constructor.
2) requestNewInstance action handling
The new overload is starting to be a “do everything” method. You can keep the same behavior but make it more readable by splitting fixed actions, custom actions, and default launch/activate into helpers.
Example:
```cpp
// private
bool DockGlobalElementModel::handleDockActions(const QString &id,
const QModelIndex &index,
const QString &action) const
{
if (action == DOCK_ACTION_DOCK) {
TaskManagerSettings::instance()
->toggleDockedElement(QStringLiteral("desktop/%1").arg(id));
return true;
}
if (action == DOCK_ACTION_FORCEQUIT) {
requestClose(index, true);
return true;
}
if (action == DOCK_ACTION_CLOSEWINDOW || action == DOCK_ACTION_CLOSEALL) {
requestClose(index, false);
return true;
}
return false;
}
bool DockGlobalElementModel::handleCustomAppAction(const QString &id,
const QString &action) const
{
if (action.isEmpty())
return false;
QProcess process;
process.setProcessChannelMode(QProcess::MergedChannels);
process.start(QStringLiteral("dde-am"), {"--by-user", id, action});
process.waitForFinished();
return true;
}
void DockGlobalElementModel::launchOrActivate(void *sourceModel,
int sourceRow,
const QString &id,
const QString &action) const
{
if (sourceModel == m_activeAppModel) {
auto model = static_cast<QAbstractItemModel *>(sourceModel);
auto sourceIndex = model->index(sourceRow, 0);
m_activeAppModel->requestNewInstance(sourceIndex, action);
return;
}
QString dbusPath =
QStringLiteral("/org/desktopspec/ApplicationManager1/")
+ escapeToObjectPath(id);
using Application = org::desktopspec::ApplicationManager1::Application;
Application appInterface(QStringLiteral("org.desktopspec.ApplicationManager1"),
dbusPath,
QDBusConnection::sessionBus());
if (appInterface.isValid()) {
appInterface.Launch(QString(), QStringList(), QVariantMap());
}
}
```
Then `requestNewInstance` is only orchestration:
```cpp
void DockGlobalElementModel::requestNewInstance(const QModelIndex &index,
const QString &action) const
{
auto data = m_data.value(index.row());
auto id = std::get<0>(data);
auto sourceModel = std::get<1>(data);
auto sourceRow = std::get<2>(data);
if (handleDockActions(id, index, action))
return;
if (handleCustomAppAction(id, action))
return;
// Default: launch/activate
launchOrActivate(sourceModel, sourceRow, id, action);
}
```
This keeps existing behavior, but it becomes much easier to extend with new actions and to test specific paths.
3) data() WinTitleRole branch
To keep `data()` from turning into a long role-specific function, you can move the window-related logic into small helpers, especially now that WinTitleRole and WindowsRole both deal with window state.
Example:
```cpp
// private
QVariant DockGlobalElementModel::dataForWindowsRole(const QString &id,
QAbstractItemModel *model,
int row) const
{
if (model == m_activeAppModel) {
return QStringList{model->index(row, 0)
.data(TaskManager::WinIdRole)
.toString()};
}
QModelIndex groupIndex = model->index(row, 0);
return groupIndex.data(TaskManager::WindowsRole).toStringList();
}
QVariant DockGlobalElementModel::dataForWinTitleRole(QAbstractItemModel *model,
int row) const
{
if (model == m_activeAppModel) {
return model->index(row, 0).data(TaskManager::WinTitleRole);
}
return {};
}
```
And in `data()`:
```cpp
switch (role) {
case TaskManager::ItemIdRole:
return id;
case TaskManager::WindowsRole:
return dataForWindowsRole(id, model, row);
case TaskManager::WinTitleRole:
return dataForWinTitleRole(model, row);
case TaskManager::MenusRole:
return getMenus(index);
default:
// existing default handling...
}
```
This keeps the switch as a simple router, and the window/title logic is localized and easier to evolve.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| void DockGlobalElementModel::requestNewInstance(const QModelIndex &index, const QString &action) const | ||
| { | ||
| auto data = m_data.value(index.row()); | ||
| auto id = std::get<0>(data); | ||
| auto sourceModel = std::get<1>(data); | ||
| auto sourceRow = std::get<2>(data); | ||
|
|
||
| // Handle special actions first (for both active and docked apps) | ||
| if (action == DOCK_ACTION_DOCK) { | ||
| TaskManagerSettings::instance()->toggleDockedElement(QStringLiteral("desktop/%1").arg(id)); |
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.
issue (performance): Avoid blocking the UI thread by waiting synchronously on QProcess when dispatching app actions.
In requestNewInstance, QProcess is started and then waitForFinished() is called, which will block the calling (likely GUI) thread until dde-am exits and can cause visible UI freezes. Instead, consider starting the process without blocking (e.g., QProcess::startDetached or an asynchronous start) and handle errors via signals or logging.
0e1e108 to
8fac4dc
Compare
1. Add window split mode to display individual windows separately in taskbar 2. Implement dynamic title calculation with adaptive character limits based on available space 3. Add new AppItemWithTitle component that displays window titles alongside icons 4. Enhance drag and drop functionality to support individual window dragging 5. Update context menu to show "Close" instead of "Close All" in split mode 6. Add WinTitleRole support in model for window title data 7. Implement intelligent window grouping logic to keep windows of same app together Log: Added window split mode showing individual window titles in taskbar Influence: 1. Test window split mode activation and deactivation 2. Verify window titles display correctly with dynamic character limits 3. Test drag and drop of individual windows in split mode 4. Check context menu shows appropriate close options 5. Verify window grouping maintains proper order 6. Test window activation and new instance creation 7. Validate title truncation and ellipsis behavior 8. Test with multiple windows of same application feat: 实现窗口分离模式并支持动态标题显示 1. 添加窗口分离模式,在任务栏中分别显示各个窗口 2. 实现基于可用空间的动态字符限制计算 3. 新增 AppItemWithTitle 组件,在图标旁显示窗口标题 4. 增强拖放功能以支持单独窗口拖动 5. 更新上下文菜单,在分离模式下显示"关闭"而非"全部关闭" 6. 在模型中添加 WinTitleRole 支持窗口标题数据 7. 实现智能窗口分组逻辑,保持同一应用的窗口在一起 Log: 新增窗口分离模式,在任务栏显示单独窗口标题 整体计算方式为: 获取任务栏的剩余范围remainingSpacesForSplitWindow,默认给所有应用窗口标题为最大宽度--7个汉字的宽度, 然后判断所有应用窗口的图标+标题总和宽度是否大于remainingSpacesForSplitWindow, 如果大于就开始缩减此时窗口的标签中最长标题的长度,(charLimits具体就是存着相对应的index存着字符的数量。)
8fac4dc to
f4a1231
Compare
deepin pr auto review我来对这段代码进行详细的审查:
// 1. 添加JSON解析错误处理
try {
model: JSON.parse(menus)
} catch(e) {
console.error("Failed to parse menus JSON:", e)
model: []
}
// 2. 添加进程超时控制
QProcess process;
process.setProcessChannelMode(QProcess::MergedChannels);
process.start("dde-am", {"--by-user", id, action});
if (!process.waitForFinished(5000)) { // 5秒超时
process.kill();
qWarning() << "dde-am process timeout";
}// 3. 优化文本计算缓存
property var textWidthCache: new Map()
function calculateTextWidth(text, textSize) {
if (!text || text.length === 0) return 0
let cacheKey = `${text}_${textSize}`
if (textWidthCache.has(cacheKey)) {
return textWidthCache.get(cacheKey)
}
textMetrics.font.pixelSize = textSize
textMetrics.text = text
let width = textMetrics.advanceWidth + 4
textWidthCache.set(cacheKey, width)
return width
}// 4. 优化延时时间
DS.singleShot(100, updateDynamicCharLimits) // 减少延时时间
这些改进建议可以帮助提高代码的健壮性、性能和可维护性。建议分阶段实施这些改进,优先处理安全性和性能相关的问题。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, wjyrich 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 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
Log: Added window split mode showing individual window titles in taskbar
feat: 实现窗口分离模式并支持动态标题显示
Log: 新增窗口分离模式,在任务栏显示单独窗口标题
整体计算方式为: 获取任务栏的剩余范围remainingSpacesForSplitWindow,默认给所有应用窗口标题为最大宽度--7个汉字的宽度, 然后判断所有应用窗口的图标+标题总和宽度是否大于remainingSpacesForSplitWindow, 如果大于就开始缩减此时窗口的标签中最长标题的长度,(charLimits具体就是存着相对应的index存着字符的数量。)
Summary by Sourcery
Introduce a window split mode in the dock task manager, showing individual windows with dynamically sized titles and updated interaction behavior.
New Features:
Enhancements:
Chores: