Fix progress bar color
ClosedPublic

Authored by antonanikin on Dec 16 2016, 7:17 AM.

Details

Summary

The patch fixes progress bar color which now is always QPalette::Highlight. This leads to invisible bar contents for selected view items when we use QStyleOptionProgressBar inside item delegates. New version fixes this issue with setting QPalette::Window color if current state has QStyle::State_Selected flag.

Old version:


New version:
with QPalette::Window


with QPalette::HighlightedText

with QPalette::Window

with QPalette::HighlightedText

Delegate code should set correct state:

void MyDelegate::paint(QPainter* painter, const QStyleOptionViewItem& option, const QModelIndex& index) const
{
    // ...
    QStyleOptionProgressBar progressBarOption;
    progressBarOption.state = option.state;
   // ...
}
Test Plan

Tested on linux system (Arch x86_64) with Qt 5.7.0

Diff Detail

Repository
R31 Breeze
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
antonanikin updated this revision to Diff 9059.Dec 16 2016, 7:17 AM
antonanikin retitled this revision from to Fix progress bar color.
antonanikin updated this object.
antonanikin edited the test plan for this revision. (Show Details)
antonanikin added a reviewer: Breeze.
antonanikin updated this object.Dec 16 2016, 7:28 AM

I agree that a fix is needed, but I am not sure that the ::Window role is the proper one.
What about "HighlightedText" ? It is supposed to be always contrasted with respect to selection color. It is also the one used for selected text and icons.
What do you think ?

antonanikin edited the summary of this revision. (Show Details)Mar 2 2017, 7:21 AM

What about "HighlightedText" ? It is supposed to be always contrasted with respect to selection color. It is also the one used for selected text and icons.

I update revison's screenshots and can't choose the best version (QPalette::Window vs QPalette::HighlightedText) :)
If you think that QPalette::HighlightedText is better I will update the revision and push it after acceptance.

Thanks for the new screenshots. I think HighlightedText is better and more consistent with the rest of the code. Maybe you can wait a day or two to see if someone else objects, (though I''m the maintainer :)), and if none, make the change.

Best,

Hugo

Maybe you can wait a day or two to see if someone else objects, (though I''m the maintainer :)), and if none, make the change.

Ok, I can wait more than 2 days :) Accept the patch, when you will be sure.

  • Use QPalette::HighlightedText instead QPalette::Window
Restricted Application added a project: Plasma. · View Herald TranscriptMar 2 2017, 10:45 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
hpereiradacosta accepted this revision.Apr 4 2017, 1:37 PM

Sorry. Forgot to accept this revision. Thanks for the updated patch. Ready to go !

This revision is now accepted and ready to land.Apr 4 2017, 1:37 PM
This revision was automatically updated to reflect the committed changes.