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 Christoph Feck


> On Oct. 30, 2014, 2:04 p.m., David Edmundson wrote:
> > src/kratingpainter.cpp, line 252
> > 
> >
> > 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


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


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


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 David Edmundson


> On Oct. 30, 2014, 2:04 p.m., David Edmundson wrote:
> > src/kratingpainter.cpp, line 231
> > 
> >
> > 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


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