Re: Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-11-16 Thread Christoph Feck

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/
---

(Updated Nov. 16, 2014, 6:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Bugs: 339863
https://bugs.kde.org/show_bug.cgi?id=339863


Repository: kwidgetsaddons


Description
---

The old code converted the yellow star icon to grayscale for the unrated part. 
With icon themes that have no color, this means the rating cannot be read, 
because all stars look identical.

This code adds support for a rating-unrated icon that theme creators can use 
for the unrated part. If the icon theme does not support this icon, it falls 
back to desaturating.


Diffs
-

  src/kratingpainter.cpp 8e8a6a3 

Diff: https://git.reviewboard.kde.org/r/120902/diff/


Testing
---

I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
dedicated rating-unrated.png file to oxygen and tested both cases (present 
and not).


Thanks,

Christoph Feck

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-11-05 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/#review69898
---

Ship it!


- David Edmundson


On Oct. 29, 2014, 11:30 p.m., Christoph Feck wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120902/
 ---
 
 (Updated Oct. 29, 2014, 11:30 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339863
 https://bugs.kde.org/show_bug.cgi?id=339863
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 The old code converted the yellow star icon to grayscale for the unrated 
 part. With icon themes that have no color, this means the rating cannot be 
 read, because all stars look identical.
 
 This code adds support for a rating-unrated icon that theme creators can 
 use for the unrated part. If the icon theme does not support this icon, it 
 falls back to desaturating.
 
 
 Diffs
 -
 
   src/kratingpainter.cpp 8e8a6a3 
 
 Diff: https://git.reviewboard.kde.org/r/120902/diff/
 
 
 Testing
 ---
 
 I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
 dedicated rating-unrated.png file to oxygen and tested both cases (present 
 and not).
 
 
 Thanks,
 
 Christoph Feck
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-10-30 Thread David Edmundson


 On Oct. 30, 2014, 2:04 p.m., David Edmundson wrote:
  src/kratingpainter.cpp, line 231
  https://git.reviewboard.kde.org/r/120902/diff/1/?file=323593#file323593line231
 
  this was previously only turned into an image to do the toGrayScale 
  conversion.
  
  I don't think we want to turn it back.

oops, ignore this.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/#review69528
---


On Oct. 29, 2014, 11:30 p.m., Christoph Feck wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120902/
 ---
 
 (Updated Oct. 29, 2014, 11:30 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339863
 https://bugs.kde.org/show_bug.cgi?id=339863
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 The old code converted the yellow star icon to grayscale for the unrated 
 part. With icon themes that have no color, this means the rating cannot be 
 read, because all stars look identical.
 
 This code adds support for a rating-unrated icon that theme creators can 
 use for the unrated part. If the icon theme does not support this icon, it 
 falls back to desaturating.
 
 
 Diffs
 -
 
   src/kratingpainter.cpp 8e8a6a3 
 
 Diff: https://git.reviewboard.kde.org/r/120902/diff/
 
 
 Testing
 ---
 
 I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
 dedicated rating-unrated.png file to oxygen and tested both cases (present 
 and not).
 
 
 Thanks,
 
 Christoph Feck
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-10-30 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/#review69528
---



src/kratingpainter.cpp
https://git.reviewboard.kde.org/r/120902/#comment48667

this was previously only turned into an image to do the toGrayScale 
conversion.

I don't think we want to turn it back.



src/kratingpainter.cpp
https://git.reviewboard.kde.org/r/120902/#comment48669

This part seems like it needs fixing too, as it's still just taking the 
full potentially grey icon and greyscaling it.


- David Edmundson


On Oct. 29, 2014, 11:30 p.m., Christoph Feck wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120902/
 ---
 
 (Updated Oct. 29, 2014, 11:30 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339863
 https://bugs.kde.org/show_bug.cgi?id=339863
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 The old code converted the yellow star icon to grayscale for the unrated 
 part. With icon themes that have no color, this means the rating cannot be 
 read, because all stars look identical.
 
 This code adds support for a rating-unrated icon that theme creators can 
 use for the unrated part. If the icon theme does not support this icon, it 
 falls back to desaturating.
 
 
 Diffs
 -
 
   src/kratingpainter.cpp 8e8a6a3 
 
 Diff: https://git.reviewboard.kde.org/r/120902/diff/
 
 
 Testing
 ---
 
 I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
 dedicated rating-unrated.png file to oxygen and tested both cases (present 
 and not).
 
 
 Thanks,
 
 Christoph Feck
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-10-30 Thread Christoph Feck


 On Oct. 30, 2014, 2:04 p.m., David Edmundson wrote:
  src/kratingpainter.cpp, line 252
  https://git.reviewboard.kde.org/r/120902/diff/1/?file=323593#file323593line252
 
  This part seems like it needs fixing too, as it's still just taking the 
  full potentially grey icon and greyscaling it.

You did not propose a suggestion :)

Note that having a theme provide a separate rating-unrated icon does not 
imply that it is grayscale.


- Christoph


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/#review69528
---


On Oct. 29, 2014, 11:30 p.m., Christoph Feck wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120902/
 ---
 
 (Updated Oct. 29, 2014, 11:30 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339863
 https://bugs.kde.org/show_bug.cgi?id=339863
 
 
 Repository: kwidgetsaddons
 
 
 Description
 ---
 
 The old code converted the yellow star icon to grayscale for the unrated 
 part. With icon themes that have no color, this means the rating cannot be 
 read, because all stars look identical.
 
 This code adds support for a rating-unrated icon that theme creators can 
 use for the unrated part. If the icon theme does not support this icon, it 
 falls back to desaturating.
 
 
 Diffs
 -
 
   src/kratingpainter.cpp 8e8a6a3 
 
 Diff: https://git.reviewboard.kde.org/r/120902/diff/
 
 
 Testing
 ---
 
 I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
 dedicated rating-unrated.png file to oxygen and tested both cases (present 
 and not).
 
 
 Thanks,
 
 Christoph Feck
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 120902: [KRatingPainter] Support monochrome icon themes (such as breeze)

2014-10-29 Thread Christoph Feck

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120902/
---

Review request for KDE Frameworks.


Bugs: 339863
https://bugs.kde.org/show_bug.cgi?id=339863


Repository: kwidgetsaddons


Description
---

The old code converted the yellow star icon to grayscale for the unrated part. 
With icon themes that have no color, this means the rating cannot be read, 
because all stars look identical.

This code adds support for a rating-unrated icon that theme creators can use 
for the unrated part. If the icon theme does not support this icon, it falls 
back to desaturating.


Diffs
-

  src/kratingpainter.cpp 8e8a6a3 

Diff: https://git.reviewboard.kde.org/r/120902/diff/


Testing
---

I couldn't actually test with breeze theme (Qt issue?), but I locally added a 
dedicated rating-unrated.png file to oxygen and tested both cases (present 
and not).


Thanks,

Christoph Feck

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel