Open
Conversation
log: In light mode, icon and text colours are 000000 with 70% opacity; in dark mode, icon and text colours are ffffff with 70% opacity; on hover, icon and text colours are 100% opacity. Furthermore, the text colour of the titles in the secondary panel does not need to be adjusted; only the colours of the icons and text in the list need to be adjusted. pms: bug-314503
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL 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 |
Reviewer's GuideUnifies quick panel icon and text hover behavior across multiple dock plugins by introducing parent-hover propagation, theme-aware text color updates with 70% default opacity and 100% on hover, and updating shared CommonIconButton colors and palettes to respect the new design in both light and dark modes. Class diagram for unified quick panel hover behaviorclassDiagram
class DFloatingButton
class QWidget
class DLabel
class DIconButton
class DGuiApplicationHelper
class QEnterEvent
class QEvent
class QIcon
class QuickButton {
+QuickButton(parent)
+void setParentHover(bool hover)
#void initStyleOption(DStyleOptionButton option)
-bool m_parentHover
}
class CommonIconButton {
+CommonIconButton(parent)
+void setIcon(QIcon icon, QColor lightColor, QColor darkColor)
+void setHoverEnable(bool enable)
+void setIconSize(QSize size)
+void setAllEnabled(bool enable)
+void setParentHover(bool hover)
-bool event(QEvent e)
-void updatePalette()
-QTimer m_refreshTimer
-bool m_clickable
-bool m_hover
-bool m_parentHover
-State m_state
-QColor m_lightThemeColor
-QColor m_darkThemeColor
-bool m_activeState
-bool m_hoverEnable
-QSize m_iconSize
}
class QuickPanelWidget_bluetooth {
+QuickPanelWidget_bluetooth(parent)
+void mousePressEvent(QMouseEvent event)
+void mouseReleaseEvent(QMouseEvent event)
+void enterEvent(QEnterEvent event)
+void leaveEvent(QEvent event)
-void initUi()
-void initConnection()
-void updateTextColor(bool hover)
-QuickButton m_iconWidget
-DLabel m_nameLabel
-DLabel m_stateLabel
-DIconButton m_expandLabel
-QPoint m_clickPoint
-bool m_hover
}
class QuickPanelWidget_wirelesscasting {
+QuickPanelWidget_wirelesscasting(parent)
+void mousePressEvent(QMouseEvent event)
+void mouseReleaseEvent(QMouseEvent event)
+void enterEvent(QEnterEvent event)
+void leaveEvent(QEvent event)
-void initUi()
-void initConnection()
-void updateTextColor(bool hover)
-QuickButton m_iconWidget
-DLabel m_nameLabel
-DLabel m_stateLabel
-DIconButton m_expandLabel
-QPoint m_clickPoint
-bool m_hover
}
class QuickPanelWidget_eyecomfort {
+QuickPanelWidget_eyecomfort(parent)
+void mousePressEvent(QMouseEvent event)
+void mouseReleaseEvent(QMouseEvent event)
+void enterEvent(QEnterEvent event)
+void leaveEvent(QEvent event)
-void initUi()
-void initConnection()
-void updateTextColor(bool hover)
-QuickButton m_iconWidget
-DLabel m_nameLabel
-DLabel m_stateLabel
-DIconButton m_expandLabel
-QPoint m_clickPoint
-bool m_eyeComfortModeEnabled
-bool m_hover
}
class QuickPanelWidget_media {
+QuickPanelWidget_media(MediaController controller, QWidget parent)
+void mouseReleaseEvent(QMouseEvent event)
+void enterEvent(QEnterEvent event)
+void leaveEvent(QEvent event)
-void init()
-void updateUI()
-void updateTextColor(bool hover)
-MediaController m_controller
-DLabel m_titleLab
-DLabel m_artistLab
-CommonIconButton m_playButton
-CommonIconButton m_nextButton
-bool m_hover
}
class SignalQuickPanel {
+SignalQuickPanel(parent)
+void setIcon(QIcon icon)
+void setDescription(QString description)
+void mouseReleaseEvent(QMouseEvent event)
+void enterEvent(QEnterEvent event)
+void leaveEvent(QEvent event)
-void initUI()
-void refreshBg()
-void updateTextColor()
-CommonIconButton m_icon
-DLabel m_description
-bool m_active
-bool m_hover
}
class JumpSettingButton {
+JumpSettingButton(QWidget parent)
+void setIcon(QIcon icon)
+void setDescription(QString description)
+bool event(QEvent e)
+void paintEvent(QPaintEvent e)
-CommonIconButton m_iconButton
-bool m_hover
}
QuickButton --|> DFloatingButton
QuickPanelWidget_bluetooth --|> QWidget
QuickPanelWidget_wirelesscasting --|> QWidget
QuickPanelWidget_eyecomfort --|> QWidget
QuickPanelWidget_media --|> QWidget
SignalQuickPanel --|> QWidget
JumpSettingButton --|> QWidget
QuickPanelWidget_bluetooth --> QuickButton
QuickPanelWidget_wirelesscasting --> QuickButton
QuickPanelWidget_eyecomfort --> QuickButton
QuickPanelWidget_media --> CommonIconButton
SignalQuickPanel --> CommonIconButton
JumpSettingButton --> CommonIconButton
CommonIconButton --> DGuiApplicationHelper
Flow diagram for hover-based text and icon color handlingflowchart TD
A_mouseEnterQuickPanel[mouseEnter QuickPanelWidget] --> B_setHoverTrue[set m_hover true]
B_setHoverTrue --> C_propagateParentHover[call setParentHover true on child buttons]
C_propagateParentHover --> D_updateTextColorHover[call updateTextColor true]
A2_mouseLeaveQuickPanel[mouseLeave QuickPanelWidget] --> B2_setHoverFalse[set m_hover false]
B2_setHoverFalse --> C2_clearParentHover[call setParentHover false on child buttons]
C2_clearParentHover --> D2_updateTextColorNormal[call updateTextColor false]
subgraph updateTextColor
D_updateTextColorHover --> E_getTheme[DGuiApplicationHelper.instance themeType]
D2_updateTextColorNormal --> E_getTheme
E_getTheme --> F_chooseBaseColor{themeType is LightType}
F_chooseBaseColor -- yes --> G_lightColor[QColor 0,0,0]
F_chooseBaseColor -- no --> H_darkColor[QColor 255,255,255]
G_lightColor --> I_setAlpha[set alpha to 1.0 when hover or 0.7 otherwise]
H_darkColor --> I_setAlpha
I_setAlpha --> J_applyPalette[update QPalette WindowText,BrightText and set on labels]
end
subgraph CommonIconButton_hover
K_enterEvent[event type Enter] --> L_setHoverTrue[set m_hover true]
K_leaveEvent[event type Leave] --> M_setHoverFalse[set m_hover false]
L_setHoverTrue --> N_updatePalette
M_setHoverFalse --> N_updatePalette
O_setParentHover[setParentHover from parent] --> P_updateParentHover[m_parentHover updated]
P_updateParentHover --> N_updatePalette
N_updatePalette --> Q_computeIconColor[theme-based color, alpha 178 normally, 255 if m_hover or m_parentHover]
Q_computeIconColor --> R_applyIconPalette[update QPalette WindowText]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The hover/text-color handling (including theme detection, alpha 0.7 vs 1.0 and nearly identical updateTextColor implementations) is duplicated across several QuickPanelWidget variants; consider extracting a small shared helper/utility or base class to avoid repeating this logic in each plugin.
- Several classes maintain a m_hover member while also passing a hover bool into updateTextColor (e.g., QuickPanelWidget in bluetooth/eye-comfort/wirelesscasting/media); since m_hover is never read inside updateTextColor, you can either drop the member or have updateTextColor read m_hover directly for clearer and less error-prone state management.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hover/text-color handling (including theme detection, alpha 0.7 vs 1.0 and nearly identical updateTextColor implementations) is duplicated across several QuickPanelWidget variants; consider extracting a small shared helper/utility or base class to avoid repeating this logic in each plugin.
- Several classes maintain a m_hover member while also passing a hover bool into updateTextColor (e.g., QuickPanelWidget in bluetooth/eye-comfort/wirelesscasting/media); since m_hover is never read inside updateTextColor, you can either drop the member or have updateTextColor read m_hover directly for clearer and less error-prone state management.
## Individual Comments
### Comment 1
<location path="plugins/dde-dock/eye-comfort-mode/quickpanelwidget.cpp" line_range="41" />
<code_context>
}
} else {
- textColor.setAlphaF(1);
+ textColor.setAlphaF(m_parentHover ? 1.0 : 0.7);
if (!option->state.testFlag(QStyle::State_Raised)) { // press
bgColor.setAlphaF(0.2);
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the hover handling by deriving hover state from `underMouse()` instead of maintaining separate `m_hover`/`m_parentHover` flags and setter wiring between parent and child widgets.
You can keep the visual behavior while simplifying the hover logic and removing redundant state.
### 1) Remove `m_parentHover` and let the button inspect its parent
Instead of wiring a separate `m_parentHover` flag through `setParentHover`, you can read the parent’s hover state directly in `QuickButton::initStyleOption`:
```cpp
void QuickButton::initStyleOption(DStyleOptionButton *option) const
{
DIconButton::initStyleOption(option);
QColor bgColor = option->palette.color(backgroundRole());
QColor textColor = option->palette.color(foregroundRole());
const bool parentHover = parentWidget() && parentWidget()->underMouse();
if (backgroundRole() == QPalette::Highlight) {
textColor = Qt::white;
if (!option->state.testFlag(QStyle::State_Raised)) {
bgColor.setHslF(bgColor.hslHueF(), bgColor.hslSaturationF(), bgColor.lightnessF() * 0.9);
textColor.setAlphaF(0.8);
} else if (option->state.testFlag(QStyle::State_MouseOver)) {
bgColor.setHslF(bgColor.hslHueF(), bgColor.hslSaturationF(), bgColor.lightnessF() * 1.1);
}
} else {
textColor.setAlphaF(parentHover ? 1.0 : 0.7);
if (!option->state.testFlag(QStyle::State_Raised)) {
bgColor.setAlphaF(0.2);
} else if (option->state.testFlag(QStyle::State_MouseOver) || parentHover) {
bgColor.setAlphaF(0.15);
} else {
bgColor.setAlphaF(0.1);
}
}
if (m_mode == ButtonMode::DisplayButton) {
bgColor.setAlphaF(0);
}
option->palette.setColor(backgroundRole(), bgColor);
option->palette.setColor(foregroundRole(), textColor);
option->state.setFlag(QStyle::State_Raised, true);
}
```
Then you can drop `QuickButton::setParentHover`, the `m_parentHover` member, and all calls to `setParentHover` in `QuickPanelWidget`.
### 2) Use a single source of truth for hover in `QuickPanelWidget`
`m_hover` and the `hover` parameter on `updateTextColor` represent the same state. You can remove both and read the hover state from `underMouse()`:
```cpp
void QuickPanelWidget::enterEvent(QEnterEvent *event)
{
updateTextColor();
QWidget::enterEvent(event);
}
void QuickPanelWidget::leaveEvent(QEvent *event)
{
updateTextColor();
QWidget::leaveEvent(event);
}
void QuickPanelWidget::updateTextColor()
{
const bool hover = underMouse();
const bool isLight = DGuiApplicationHelper::instance()->themeType()
== DGuiApplicationHelper::LightType;
QColor color = isLight ? QColor(0, 0, 0) : QColor(255, 255, 255);
color.setAlphaF(hover ? 1.0 : 0.7);
QPalette pa = m_nameLabel->palette();
pa.setColor(QPalette::BrightText, color);
pa.setColor(QPalette::WindowText, color);
m_nameLabel->setPalette(pa);
m_stateLabel->setPalette(pa);
}
```
With that, you can remove:
- The `m_hover` member.
- The `bool hover` parameter from `updateTextColor` and all call sites passing `true`/`false`.
- The `setParentHover(true/false)` calls (since `QuickButton` now reads `parentWidget()->underMouse()`).
This keeps all visual behavior but:
- Eliminates redundant state (`m_hover`, `m_parentHover`).
- Removes flag juggling between parent and child.
- Makes the hover behavior more self-contained and easier to reason about.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
| } else { | ||
| textColor.setAlphaF(1); | ||
| textColor.setAlphaF(m_parentHover ? 1.0 : 0.7); |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the hover handling by deriving hover state from underMouse() instead of maintaining separate m_hover/m_parentHover flags and setter wiring between parent and child widgets.
You can keep the visual behavior while simplifying the hover logic and removing redundant state.
1) Remove m_parentHover and let the button inspect its parent
Instead of wiring a separate m_parentHover flag through setParentHover, you can read the parent’s hover state directly in QuickButton::initStyleOption:
void QuickButton::initStyleOption(DStyleOptionButton *option) const
{
DIconButton::initStyleOption(option);
QColor bgColor = option->palette.color(backgroundRole());
QColor textColor = option->palette.color(foregroundRole());
const bool parentHover = parentWidget() && parentWidget()->underMouse();
if (backgroundRole() == QPalette::Highlight) {
textColor = Qt::white;
if (!option->state.testFlag(QStyle::State_Raised)) {
bgColor.setHslF(bgColor.hslHueF(), bgColor.hslSaturationF(), bgColor.lightnessF() * 0.9);
textColor.setAlphaF(0.8);
} else if (option->state.testFlag(QStyle::State_MouseOver)) {
bgColor.setHslF(bgColor.hslHueF(), bgColor.hslSaturationF(), bgColor.lightnessF() * 1.1);
}
} else {
textColor.setAlphaF(parentHover ? 1.0 : 0.7);
if (!option->state.testFlag(QStyle::State_Raised)) {
bgColor.setAlphaF(0.2);
} else if (option->state.testFlag(QStyle::State_MouseOver) || parentHover) {
bgColor.setAlphaF(0.15);
} else {
bgColor.setAlphaF(0.1);
}
}
if (m_mode == ButtonMode::DisplayButton) {
bgColor.setAlphaF(0);
}
option->palette.setColor(backgroundRole(), bgColor);
option->palette.setColor(foregroundRole(), textColor);
option->state.setFlag(QStyle::State_Raised, true);
}Then you can drop QuickButton::setParentHover, the m_parentHover member, and all calls to setParentHover in QuickPanelWidget.
2) Use a single source of truth for hover in QuickPanelWidget
m_hover and the hover parameter on updateTextColor represent the same state. You can remove both and read the hover state from underMouse():
void QuickPanelWidget::enterEvent(QEnterEvent *event)
{
updateTextColor();
QWidget::enterEvent(event);
}
void QuickPanelWidget::leaveEvent(QEvent *event)
{
updateTextColor();
QWidget::leaveEvent(event);
}
void QuickPanelWidget::updateTextColor()
{
const bool hover = underMouse();
const bool isLight = DGuiApplicationHelper::instance()->themeType()
== DGuiApplicationHelper::LightType;
QColor color = isLight ? QColor(0, 0, 0) : QColor(255, 255, 255);
color.setAlphaF(hover ? 1.0 : 0.7);
QPalette pa = m_nameLabel->palette();
pa.setColor(QPalette::BrightText, color);
pa.setColor(QPalette::WindowText, color);
m_nameLabel->setPalette(pa);
m_stateLabel->setPalette(pa);
}With that, you can remove:
- The
m_hovermember. - The
bool hoverparameter fromupdateTextColorand all call sites passingtrue/false. - The
setParentHover(true/false)calls (sinceQuickButtonnow readsparentWidget()->underMouse()).
This keeps all visual behavior but:
- Eliminates redundant state (
m_hover,m_parentHover). - Removes flag juggling between parent and child.
- Makes the hover behavior more self-contained and easier to reason about.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
log: In light mode, icon and text colours are 000000 with 70% opacity; in dark mode, icon and text colours are ffffff with 70% opacity; on hover, icon and text colours are 100% opacity. Furthermore, the text colour of the titles in the secondary panel does not need to be adjusted; only the colours of the icons and text in the list need to be adjusted.
pms: bug-314503
Summary by Sourcery
Align quick panel list icon and text colors with design requirements across dock plugins, including hover behavior and light/dark theme opacity.
Bug Fixes:
Enhancements: