Feat: add HeaderInfoTableWidget for displaying device information#632
Conversation
Add a custom table widget with rounded border and alternating row colors for displaying device information as key-value pairs. - Add HeaderInfoDelegate for custom text color handling in selection - Add HeaderInfoTableWidget with rounded border painting and vertical divider - Support dynamic height adjustment based on content - Use DPalette for consistent theming with active/inactive states Log: add feature for cpu info show. Task: https://pms.uniontech.com/task-view-387697.html
Reviewer's GuideImplements a new HeaderInfoTableWidget and HeaderInfoDelegate to display key-value device information in a non-editable, styled, two-column table with rounded borders, alternating row colors, theming via DPalette, and dynamic height sizing based on content. Sequence diagram for HeaderInfoDelegate paint with themingsequenceDiagram
participant View
participant Delegate
participant Application
participant PaletteHelper
View->>Delegate: paint(painter, option, index)
Delegate->>Application: activeWindow()
Application-->>Delegate: activeWindow_or_null
Delegate->>PaletteHelper: palette(option.widget)
PaletteHelper-->>Delegate: palette
alt item_selected
Delegate->>Delegate: set Text and WindowText to HighlightedText
else item_not_selected
Delegate->>Delegate: set Text and WindowText to Text
end
Delegate->>View: QStyledItemDelegate::paint(painter, option, index)
Class diagram for HeaderInfoTableWidget and HeaderInfoDelegateclassDiagram
class DTableWidget
class DWidget
class QStyledItemDelegate
class QObject
class HeaderInfoTableWidget {
+HeaderInfoTableWidget(DWidget* parent)
+void updateData(QList_QPair data)
#void paintEvent(QPaintEvent* event)
-void initUI()
-void clear()
-HeaderInfoDelegate* m_delegate
}
class HeaderInfoDelegate {
+HeaderInfoDelegate(QObject* parent)
+void paint(QPainter* painter, QStyleOptionViewItem option, QModelIndex index)
}
HeaderInfoTableWidget --|> DTableWidget
HeaderInfoDelegate --|> QStyledItemDelegate
HeaderInfoTableWidget o--> HeaderInfoDelegate
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Redefining
clear()as a private helper inHeaderInfoTableWidgethides the publicQTableWidget::clear()API; consider renaming this helper (e.g.resetTable()) to avoid surprising callers who may expect to accessclear(). - The vertical divider in
paintEventuses a hard-coded x-coordinate (179) while the first column width is configured separately; it would be more robust to compute the divider position from the header (e.g.horizontalHeader()->sectionPosition(0) + horizontalHeader()->sectionSize(0)) to keep them in sync if the column width changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Redefining `clear()` as a private helper in `HeaderInfoTableWidget` hides the public `QTableWidget::clear()` API; consider renaming this helper (e.g. `resetTable()`) to avoid surprising callers who may expect to access `clear()`.
- The vertical divider in `paintEvent` uses a hard-coded x-coordinate (`179`) while the first column width is configured separately; it would be more robust to compute the divider position from the header (e.g. `horizontalHeader()->sectionPosition(0) + horizontalHeader()->sectionSize(0)`) to keep them in sync if the column width changes.
## Individual Comments
### Comment 1
<location path="deepin-devicemanager/src/Widget/headerinfotablewidget.cpp" line_range="117-118" />
<code_context>
+ painter.drawRoundedRect(rectIn, radius, radius);
+
+ // Draw vertical divider between first and second columns (same as DetailTreeView)
+ QLine vline(rect.topLeft().x() + 179, rect.topLeft().y(), rect.bottomLeft().x() + 179, rect.bottomLeft().y());
+ painter.drawLine(vline);
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Hardcoded divider position will desync if the column width changes (resize, DPI, style).
The X-position is hardcoded to 179 while the first column width is 180 and can change (resize, DPI, layout). When that happens, the divider won’t align with the column border. Please compute the position dynamically (e.g., `columnViewportPosition(0) + columnWidth(0)`) so it always matches the actual column boundary.
</issue_to_address>
### Comment 2
<location path="deepin-devicemanager/src/Widget/headerinfotablewidget.cpp" line_range="26" />
<code_context>
+ initUI();
+}
+
+void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data)
+{
+ clear();
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring palette handling, geometry calculations, and the `clear()` helper to remove duplication, magic numbers, and surprising overrides while keeping the widget’s behavior unchanged.
You can reduce complexity and keep behavior by extracting the repeated palette logic, deriving geometry from the table state, and clarifying the `clear()` override.
### 1. Centralize palette/color-group logic
`updateData()` and `paintEvent()` both compute the active window, color group, and palette. Extract this into a small helper:
```cpp
// in headerinfotablewidget.h (private):
DPalette paletteForCurrentState() const;
DPalette::ColorGroup colorGroupForCurrentState() const;
```
```cpp
// in headerinfotablewidget.cpp
DPalette::ColorGroup HeaderInfoTableWidget::colorGroupForCurrentState() const
{
QWidget *wnd = DApplication::activeWindow();
return wnd ? DPalette::Active : DPalette::Inactive;
}
DPalette HeaderInfoTableWidget::paletteForCurrentState() const
{
return DPaletteHelper::instance()->palette(this);
}
```
Then use it in both places:
```cpp
void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data)
{
resetTableContents(); // see section 3
const int nRow = data.size();
setRowCount(nRow);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
const QColor colorEven = palette.color(cg, DPalette::Base);
const QColor colorOdd = palette.color(cg, DPalette::ItemBackground);
// ...
}
```
```cpp
void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
// ...
QBrush bgBrush(palette.color(cg, DPalette::FrameBorder));
// ...
}
```
This keeps the color behavior identical but removes duplication and makes future palette changes localized.
### 2. Derive divider position from header instead of magic constant
The `179` constant is implicitly tied to the header’s default section size of `180`. Use the header API to compute the divider, so column width changes stay in sync:
```cpp
void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
QRect rect = viewport()->rect().adjusted(0, 0, -1, -1);
const int width = 1;
const int radius = 8;
QPainter painter(viewport());
painter.setRenderHint(QPainter::Antialiasing, true);
// ... border drawing as before ...
// Derive divider X from first column width
int firstColumnWidth = horizontalHeader()->sectionSize(0);
int dividerX = rect.left() + firstColumnWidth - 1; // keep visual alignment similar
QLine vline(dividerX, rect.top(), dividerX, rect.bottom());
painter.drawLine(vline);
}
```
This preserves the look while removing the hard-coded coupling between `179` and `180`.
### 3. Clarify and avoid shadowing `clear()`
Your private `clear()` shadows `QTableWidget::clear()` and adds `setRowCount(0)`. To make intent clearer and reduce surprise, rename and use a more descriptive helper:
```cpp
// headerinfotablewidget.h (private)
void resetTableContents();
```
```cpp
// headerinfotablewidget.cpp
void HeaderInfoTableWidget::resetTableContents()
{
DTableWidget::clear();
setRowCount(0);
}
```
Then in `updateData()`:
```cpp
void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data)
{
resetTableContents();
// ...
}
```
This keeps the exact behavior but avoids shadowing the base `clear()` and makes the reset semantics explicit.
### 4. Optionally extract border drawing into a helper
If you want to further simplify `paintEvent()`, move the border logic into a separate private method, keeping `paintEvent()` focused:
```cpp
// headerinfotablewidget.h (private):
void drawRoundedBorderAndDivider(QPainter &painter, const QRect &rect,
const DPalette &palette, DPalette::ColorGroup cg);
```
```cpp
void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
QRect rect = viewport()->rect().adjusted(0, 0, -1, -1);
QPainter painter(viewport());
painter.setRenderHint(QPainter::Antialiasing, true);
drawRoundedBorderAndDivider(painter, rect, palette, cg);
}
void HeaderInfoTableWidget::drawRoundedBorderAndDivider(
QPainter &painter, const QRect &rect,
const DPalette &palette, DPalette::ColorGroup cg)
{
const int width = 1;
const int radius = 8;
QPainterPath outer;
outer.addRoundedRect(rect, radius, radius);
QRect inner(rect.adjusted(width, width, -width, -width));
QPainterPath innerPath;
innerPath.addRoundedRect(inner, radius, radius);
QPainterPath borderPath = outer.subtracted(innerPath);
painter.fillPath(borderPath, palette.color(cg, DPalette::FrameBorder));
QPen pen = painter.pen();
pen.setWidth(width);
pen.setColor(palette.color(cg, DPalette::FrameBorder));
painter.setPen(pen);
painter.drawRoundedRect(inner, radius, radius);
int firstColumnWidth = horizontalHeader()->sectionSize(0);
int dividerX = rect.left() + firstColumnWidth - 1;
painter.drawLine(dividerX, rect.top(), dividerX, rect.bottom());
}
```
This keeps all visuals intact while making `paintEvent()` much easier to read and maintain.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QLine vline(rect.topLeft().x() + 179, rect.topLeft().y(), rect.bottomLeft().x() + 179, rect.bottomLeft().y()); | ||
| painter.drawLine(vline); |
There was a problem hiding this comment.
issue (bug_risk): Hardcoded divider position will desync if the column width changes (resize, DPI, style).
The X-position is hardcoded to 179 while the first column width is 180 and can change (resize, DPI, layout). When that happens, the divider won’t align with the column border. Please compute the position dynamically (e.g., columnViewportPosition(0) + columnWidth(0)) so it always matches the actual column boundary.
| initUI(); | ||
| } | ||
|
|
||
| void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data) |
There was a problem hiding this comment.
issue (complexity): Consider refactoring palette handling, geometry calculations, and the clear() helper to remove duplication, magic numbers, and surprising overrides while keeping the widget’s behavior unchanged.
You can reduce complexity and keep behavior by extracting the repeated palette logic, deriving geometry from the table state, and clarifying the clear() override.
1. Centralize palette/color-group logic
updateData() and paintEvent() both compute the active window, color group, and palette. Extract this into a small helper:
// in headerinfotablewidget.h (private):
DPalette paletteForCurrentState() const;
DPalette::ColorGroup colorGroupForCurrentState() const;// in headerinfotablewidget.cpp
DPalette::ColorGroup HeaderInfoTableWidget::colorGroupForCurrentState() const
{
QWidget *wnd = DApplication::activeWindow();
return wnd ? DPalette::Active : DPalette::Inactive;
}
DPalette HeaderInfoTableWidget::paletteForCurrentState() const
{
return DPaletteHelper::instance()->palette(this);
}Then use it in both places:
void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data)
{
resetTableContents(); // see section 3
const int nRow = data.size();
setRowCount(nRow);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
const QColor colorEven = palette.color(cg, DPalette::Base);
const QColor colorOdd = palette.color(cg, DPalette::ItemBackground);
// ...
}void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
// ...
QBrush bgBrush(palette.color(cg, DPalette::FrameBorder));
// ...
}This keeps the color behavior identical but removes duplication and makes future palette changes localized.
2. Derive divider position from header instead of magic constant
The 179 constant is implicitly tied to the header’s default section size of 180. Use the header API to compute the divider, so column width changes stay in sync:
void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
QRect rect = viewport()->rect().adjusted(0, 0, -1, -1);
const int width = 1;
const int radius = 8;
QPainter painter(viewport());
painter.setRenderHint(QPainter::Antialiasing, true);
// ... border drawing as before ...
// Derive divider X from first column width
int firstColumnWidth = horizontalHeader()->sectionSize(0);
int dividerX = rect.left() + firstColumnWidth - 1; // keep visual alignment similar
QLine vline(dividerX, rect.top(), dividerX, rect.bottom());
painter.drawLine(vline);
}This preserves the look while removing the hard-coded coupling between 179 and 180.
3. Clarify and avoid shadowing clear()
Your private clear() shadows QTableWidget::clear() and adds setRowCount(0). To make intent clearer and reduce surprise, rename and use a more descriptive helper:
// headerinfotablewidget.h (private)
void resetTableContents();// headerinfotablewidget.cpp
void HeaderInfoTableWidget::resetTableContents()
{
DTableWidget::clear();
setRowCount(0);
}Then in updateData():
void HeaderInfoTableWidget::updateData(const QList<QPair<QString, QString>> &data)
{
resetTableContents();
// ...
}This keeps the exact behavior but avoids shadowing the base clear() and makes the reset semantics explicit.
4. Optionally extract border drawing into a helper
If you want to further simplify paintEvent(), move the border logic into a separate private method, keeping paintEvent() focused:
// headerinfotablewidget.h (private):
void drawRoundedBorderAndDivider(QPainter &painter, const QRect &rect,
const DPalette &palette, DPalette::ColorGroup cg);void HeaderInfoTableWidget::paintEvent(QPaintEvent *event)
{
DTableWidget::paintEvent(event);
const auto cg = colorGroupForCurrentState();
const auto palette = paletteForCurrentState();
QRect rect = viewport()->rect().adjusted(0, 0, -1, -1);
QPainter painter(viewport());
painter.setRenderHint(QPainter::Antialiasing, true);
drawRoundedBorderAndDivider(painter, rect, palette, cg);
}
void HeaderInfoTableWidget::drawRoundedBorderAndDivider(
QPainter &painter, const QRect &rect,
const DPalette &palette, DPalette::ColorGroup cg)
{
const int width = 1;
const int radius = 8;
QPainterPath outer;
outer.addRoundedRect(rect, radius, radius);
QRect inner(rect.adjusted(width, width, -width, -width));
QPainterPath innerPath;
innerPath.addRoundedRect(inner, radius, radius);
QPainterPath borderPath = outer.subtracted(innerPath);
painter.fillPath(borderPath, palette.color(cg, DPalette::FrameBorder));
QPen pen = painter.pen();
pen.setWidth(width);
pen.setColor(palette.color(cg, DPalette::FrameBorder));
painter.setPen(pen);
painter.drawRoundedRect(inner, radius, radius);
int firstColumnWidth = horizontalHeader()->sectionSize(0);
int dividerX = rect.left() + firstColumnWidth - 1;
painter.drawLine(dividerX, rect.top(), dividerX, rect.bottom());
}This keeps all visuals intact while making paintEvent() much easier to read and maintain.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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: unstable) |
e5f236f
into
linuxdeepin:develop/eagle-20260325
Add a custom table widget with rounded border and alternating row colors for displaying device information as key-value pairs.
Log: add feature for cpu info show.
Task: https://pms.uniontech.com/task-view-387697.html
Summary by Sourcery
Introduce a themed header information table widget for displaying device key–value data with custom rendering and layout.
New Features:
Enhancements: