Bug 429972 - Buttons/ToolButtons in ActionToolBars don't respect Theme.colorSet and Theme.inherit
Summary: Buttons/ToolButtons in ActionToolBars don't respect Theme.colorSet and Theme....
Status: REOPENED
Alias: None
Product: frameworks-kirigami
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: Master
Platform: Other Linux
: NOR normal
Target Milestone: Not decided
Assignee: Marco Martin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-03 14:40 UTC by Noah Davis
Modified: 2020-12-13 22:53 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Correct appearance for qqc2-breeze-style (11.78 KB, image/png)
2020-12-03 14:41 UTC, Noah Davis
Details
Incorrect appearance for qqc2-breeze-style (12.09 KB, image/png)
2020-12-03 14:43 UTC, Noah Davis
Details
Material: Correct (top) and incorrect (bottom) (32.57 KB, image/png)
2020-12-03 14:46 UTC, Noah Davis
Details
Default: Incorrect (top) and correct (bottom) (35.18 KB, image/png)
2020-12-03 14:49 UTC, Noah Davis
Details
Colors only correct on first press with qqc2-breeze-style (535.58 KB, video/webm)
2020-12-13 22:53 UTC, Noah Davis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Noah Davis 2020-12-03 14:40:55 UTC
SUMMARY
They will always have Theme.colorSet forced to Header and Theme.inherit forced to true.

To be clear, this with all ActionToolBars, not just the global toolbar.

This problem also happens with the Default, Material and Fusion styles.

I figured out a workaround, but it's awful: https://invent.kde.org/plasma/qqc2-breeze-style/-/blob/master/style/qtquickcontrols/ToolButton.qml

STEPS TO REPRODUCE
1. Comment out lines 26-29
2. Compile qqc2-breeze-style
3. Run `QT_QUICK_CONTROLS_STYLE=org.kde.breeze kirigami2gallery`
4. Click on buttons in the global toolbar.

OBSERVED RESULT
Colors for toolbars and buttons on toolbars are wrong.

If you use console.log(), inherit is always true and colorSet is always 6 (Theme.Header)

EXPECTED RESULT
Colors should match whatever the QQC2 style would normally use.

SOFTWARE/OS VERSIONS
Operating System: openSUSE Tumbleweed 20201201
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.77.0
Qt Version: 5.15.2
Kernel Version: 5.9.8-2-default
OS Type: 64-bit
Processors: 4 × Intel® Core™ i7-6500U CPU @ 2.50GHz
Memory: 7.6 GiB of RAM
Graphics Processor: Mesa DRI Intel® HD Graphics 520
Comment 1 Noah Davis 2020-12-03 14:41:33 UTC
Created attachment 133834 [details]
Correct appearance for qqc2-breeze-style
Comment 2 Noah Davis 2020-12-03 14:43:16 UTC
Created attachment 133835 [details]
Incorrect appearance for qqc2-breeze-style
Comment 3 Noah Davis 2020-12-03 14:46:08 UTC
Created attachment 133836 [details]
Material: Correct (top) and incorrect (bottom)

The incorrect style looks better and closer to modern Android, but it's still incorrect.
Comment 4 Noah Davis 2020-12-03 14:49:36 UTC
Created attachment 133837 [details]
Default: Incorrect (top) and correct (bottom)
Comment 5 Noah Davis 2020-12-03 14:50:39 UTC
Whoops, I named the Material screenshot incorrectly. Top is incorrect and bottom is correct.
Comment 6 Noah Davis 2020-12-03 14:53:03 UTC
Also, you can't actually see the problem in the Fusion style since it hardcodes more of its button appearance and uses window color for toolbars (except for the gradient, but Kirigami doesn't use QQC2 ToolBars), so disregard what I said about Fusion.
Comment 7 Nate Graham 2020-12-03 21:03:27 UTC
Theme.inherit being true seems like a bug. If you force the color set to something non-default, you're suppose to turn off inheritance, not force it to on. The results will be nonsensical, as you're reporting.

I wonder if this is what's causing Bug 429399.
Comment 8 Bug Janitor Service 2020-12-09 14:31:00 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kirigami/-/merge_requests/184
Comment 9 Marco Martin 2020-12-09 15:08:18 UTC
Git commit fe101efd201255a61e47f387b2aa6d59da829f7c by Marco Martin.
Committed on 09/12/2020 at 15:07.
Pushed by mart into branch 'master'.

Color icons, not buttons

when an icon color is set, it tried to color the button and the icon
causing often ureadable results. now color just the icon but not
the button, giving a less magic behavior. it won't be possible
to color non monochrome icons, but that's kinda expected and should
be an acceptable compromise
Related: bug 429399

M  +2    -2    src/controls/ActionToolBar.qml
M  +0    -6    src/controls/private/PrivateActionToolButton.qml

https://invent.kde.org/frameworks/kirigami/commit/fe101efd201255a61e47f387b2aa6d59da829f7c
Comment 10 Noah Davis 2020-12-13 22:49:53 UTC
Unfortunately, that commit didn't completely fix the issue. With qqc2-breeze-style, the buttons have the correct pressed color on the first press, but not on any press after that.
Comment 11 Noah Davis 2020-12-13 22:53:37 UTC
Created attachment 134061 [details]
Colors only correct on first press with qqc2-breeze-style