Differential D8031

Fix icon colors of inline context buttons also in full screen mode
ClosedPublic
Actions

Authored by rkflx on Sep 28 2017, 1:46 PM.

Details

Reviewers
broulik
ngraham
Group Reviewers
KDE Applications
Commits
R260:46e95531c731: Fix icon colors of inline context buttons also in full screen mode
Summary

In the darker full screen mode, the icons of the mouse-over overlays
were still hard to see against the similarly coloured background. This
was because when switching to full screen mode, we would miss to reset
the icon (except for mToggleSelectionButton). The root cause is
QAbstractButton, which just holds onto a pixmap and has no way of
knowing KIconLoader would return a differently coloured icon when the
colour palette changes (as is the case for colour-aware icon sets like
"Breeze").

To fix this, we simply use QIcon::fromTheme instead of SmallIcon,
which returns a proper QIcon instead of a QPixmap and automatically
adapts to colour palette changes.

(This is a followup patch to D7988.)

CCBUG: 383059
FIXED-IN: 17.08.2

Test Plan

Overlay icons perfectly visible against the background color in standard
and full screen mode. Tested with "Breeze" and "Breeze Dark" colour
schemes as well as "Breeze", "Oxygen" and "Fusion" widget styles.
("Fusion" does not allow to customize the size for small icons, though.)

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx created this revision.Sep 28 2017, 1:46 PM
broulik added a comment.Sep 28 2017, 1:50 PM

Will using QIcon::fromTheme instead of SmallIcon (which returns a QPixmap not a QIcon) help here? QIcon also stores the name of the icon.

rkflx updated this revision to Diff 20048.EditedSep 28 2017, 4:42 PM
rkflx edited the summary of this revision. (Show Details)
rkflx edited the test plan for this revision. (Show Details)

Implemented your suggestion:

QIcon::fromTheme instead of SmallIcon

Seems like this returns a "small" icon by default, so it works as before when changing the size for small icons in the KCM.

QIcon also stores the name of the icon

Not only that, it also updates itself automatically when the colour palette changes. This makes the patch even simpler, thanks!

(And we can keep @ngraham's changes...)

ngraham added a comment.Sep 28 2017, 4:46 PM

Lovely. Will test and review later today, if it hasn't already been committed by then.

broulik accepted this revision.Sep 28 2017, 5:54 PM

Lovely :)

This revision is now accepted and ready to land.Sep 28 2017, 5:54 PM
Closed by commit R260:46e95531c731: Fix icon colors of inline context buttons also in full screen mode (authored by rkflx). · Explain WhySep 29 2017, 6:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20116

View Options

lib/datewidget.cpp

Loading...
View Options

lib/thumbnailview/previewitemdelegate.cpp

Loading...
View Options

lib/thumbnailview/thumbnailbarview.cpp

Loading...