D7318: KFileItemDelegate: Always reserve space for icons

2017-09-19 Thread Fabian Vogt
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:31361267d419: KFileItemDelegate: Always reserve space for 
icons (authored by fvogt).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7318?vs=18165=19665

REVISION DETAIL
  https://phabricator.kde.org/D7318

AFFECTED FILES
  src/widgets/kfileitemdelegate.cpp

To: fvogt, #frameworks
Cc: cfeck, #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-09-16 Thread Fabian Vogt
fvogt added a comment.


  Ping #2.
  
  If no reply until monday I'll assume this is ok.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-08-22 Thread Fabian Vogt
fvogt added a comment.


  Ping?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-08-15 Thread Fabian Vogt
fvogt added a comment.


  The underlying issue is that KIconLoader's implementation of isNull (in 
virtual_hook) is not really meaningful.
  In cases where a name for a nonexistent icon is passed, pixmap will draw the 
"unknown" icon, but isNull still returns true.
  
  So the question is whether this is intentional or not. The API doesn't really 
make that clear.
  
  If isNull should return false iff pixmap returns something other than an 
empty pixmap, the implementation for KIconLoader
  would probably always return false...
  
  In any case, I still think this diff isn't wrong by itself as the drawing of 
the pixmap is still unconditional and so should be reflected
  in the text margin.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-08-15 Thread Christoph Feck
cfeck added a comment.


  BUG: 372207

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: cfeck, #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-08-15 Thread Fabian Vogt
fvogt added a comment.


  It would be possible to avoid drawing null QIcons as well, but that would 
result in broken alignment.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7318

To: fvogt, #frameworks
Cc: #frameworks


D7318: KFileItemDelegate: Always reserve space for icons

2017-08-15 Thread Fabian Vogt
fvogt created this revision.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  Even null icons are drawn so also reserve space for them to avoid overlapping.

TEST PLAN
  http://imgur.com/a/fG2T0

REPOSITORY
  R241 KIO

BRANCH
  iconfix-master

REVISION DETAIL
  https://phabricator.kde.org/D7318

AFFECTED FILES
  src/widgets/kfileitemdelegate.cpp

To: fvogt, #frameworks
Cc: #frameworks