Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-13 Thread Marco Martin 

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

(Updated May 13, 2016, 9:51 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 53a775f12ced15ffdafef307e5ef5afe5322fb73 by Marco Martin 
to branch master.


Repository: kio


Description
---

since now kiconloader can color the icons in the sidebar, by using the 
"selected" icon mode, when the breeze icon theme is used the icon of the 
current sidebar item will be of the same color of selected text (white-ish by 
default), is more in line with breeze style and the monochrome icons become 
more readable


Diffs
-

  src/filewidgets/kfileplacesview.cpp 03074e0 
  src/widgets/kfileitemdelegate.cpp 1516b9e 

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


Testing
---


File Attachments


menu3.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png


Thanks,

Marco Martin

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread David Edmundson 

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


Ship it!




Ship It!

- David Edmundson


On May 12, 2016, 9:53 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 12, 2016, 9:53 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread Marco Martin 

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

(Updated May 12, 2016, 9:53 a.m.)


Review request for KDE Frameworks and Plasma.


Changes
---

similar logic in the behavior as qcommonstyle


Repository: kio


Description
---

since now kiconloader can color the icons in the sidebar, by using the 
"selected" icon mode, when the breeze icon theme is used the icon of the 
current sidebar item will be of the same color of selected text (white-ish by 
default), is more in line with breeze style and the monochrome icons become 
more readable


Diffs (updated)
-

  src/filewidgets/kfileplacesview.cpp 03074e0 
  src/widgets/kfileitemdelegate.cpp 1516b9e 

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


Testing
---


File Attachments


menu3.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png


Thanks,

Marco Martin

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-12 Thread Marco Martin 


> On May 11, 2016, 11:03 a.m., David Edmundson wrote:
> > src/widgets/kfileitemdelegate.cpp, line 1220
> > 
> >
> > QCommonStyle when rendering CE_ItemViewItem checks for disabled first
> > 
> > QIcon::Mode mode = QIcon::Normal;
> > if (!(vopt->state & QStyle::State_Enabled))
> > mode = QIcon::Disabled;
> > else if (vopt->state & QStyle::State_Selected)
> > mode = QIcon::Selected;
> > 
> > qcommonstyle:2167
> > 
> > it's probably best to emulate that
> > 
> > 
> > also why do you have the && active?
> > 
> > That means it'd need to be both selected and have mouse over at the 
> > same time in order to change icon?

yes, it changes the color only on active palette, changing it on inactive 
palette doesn't look really good.

unfortunately can't really pass the palette state to the iconloader, so when 
recoloring is always using the active palette, not doing the invert (that would 
be with active colors) on inactive palette seems to be the way that looks less 
bad


- Marco


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


On May 10, 2016, 11:57 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 10, 2016, 11:57 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-11 Thread David Edmundson 

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




src/widgets/kfileitemdelegate.cpp (line 1220)


QCommonStyle when rendering CE_ItemViewItem checks for disabled first

QIcon::Mode mode = QIcon::Normal;
if (!(vopt->state & QStyle::State_Enabled))
mode = QIcon::Disabled;
else if (vopt->state & QStyle::State_Selected)
mode = QIcon::Selected;

qcommonstyle:2167

it's probably best to emulate that

also why do you have the && active?

That means it'd need to be both selected and have mouse over at the same 
time in order to change icon?


- David Edmundson


On May 10, 2016, 11:57 a.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 10, 2016, 11:57 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-10 Thread Emmanuel Pescosta 

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




src/filewidgets/kfileplacesview.cpp (line 157)


option.state & (QStyle::State_Selected | QStyle::State_Active) maybe?



src/widgets/kfileitemdelegate.cpp (line 1220)


option.state & (QStyle::State_Selected | QStyle::State_Active) maybe?


- Emmanuel Pescosta


On May 10, 2016, 1:57 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated May 10, 2016, 1:57 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
>   src/widgets/kfileitemdelegate.cpp 1516b9e 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-10 Thread Marco Martin 

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

(Updated May 10, 2016, 11:57 a.m.)


Review request for KDE Frameworks and Plasma.


Repository: kio


Description
---

since now kiconloader can color the icons in the sidebar, by using the 
"selected" icon mode, when the breeze icon theme is used the icon of the 
current sidebar item will be of the same color of selected text (white-ish by 
default), is more in line with breeze style and the monochrome icons become 
more readable


Diffs (updated)
-

  src/filewidgets/kfileplacesview.cpp 03074e0 
  src/widgets/kfileitemdelegate.cpp 1516b9e 

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


Testing
---


File Attachments


menu3.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png


Thanks,

Marco Martin

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

Re: Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-09 Thread Andreas Kainz 

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


Ship it!




I can't review the code but this is one of the best features in the past 2 years

- Andreas Kainz


On Mai 9, 2016, 4:24 nachm., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127878/
> ---
> 
> (Updated Mai 9, 2016, 4:24 nachm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> since now kiconloader can color the icons in the sidebar, by using the 
> "selected" icon mode, when the breeze icon theme is used the icon of the 
> current sidebar item will be of the same color of selected text (white-ish by 
> default), is more in line with breeze style and the monochrome icons become 
> more readable
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfileplacesview.cpp 03074e0 
> 
> Diff: https://git.reviewboard.kde.org/r/127878/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> menu3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Review Request 127878: use the "selected" icon mode in file open dialog sidebar

2016-05-09 Thread Marco Martin 

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

Review request for KDE Frameworks and Plasma.


Repository: kio


Description
---

since now kiconloader can color the icons in the sidebar, by using the 
"selected" icon mode, when the breeze icon theme is used the icon of the 
current sidebar item will be of the same color of selected text (white-ish by 
default), is more in line with breeze style and the monochrome icons become 
more readable


Diffs
-

  src/filewidgets/kfileplacesview.cpp 03074e0 

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


Testing
---


File Attachments


menu3.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/05/09/41f52d86-6fc5-4799-bd9e-18d17dc5a65a__menu3.png


Thanks,

Marco Martin

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