D8243: Implement support for categories on KfilesPlacesView

[Kevin Ottens]

ervin added a comment.


  A couple more changes needed.

INLINE COMMENTS

> kfileplacesitem.cpp:153-158
> +    if (role == KFilePlacesModel::GroupRole) {
> +        return QVariant(m_groupName);
> +    }
>  
> +    QVariant returnData;
>      if (role != KFilePlacesModel::HiddenRole && role != Qt::BackgroundRole && isDevice()) {

Nitpicking a bit (and sorry for not realizing it earlier) but I think I'd try to keep the structure of the method before that patch and so go for:

  if (role == KFilePlacesModel::GroupRole)
  ...
  else if (role != KFilePlacesModel::HiddenRole ...)
  ...
  else

Probably returnData can go away though and just return from the branches of the if instead.

> kfileplacesview.cpp:108
> +
> +    mutable int m_dragModeCount;
> +

OK, for that one you can probably get away with a bool instead (let's call it m_dragStarted maybe?). The counter was for a more generic case of more than one item being draggable at the same time, but that won't be the case here since it's a single selection list.

> anthonyfieroni wrote in kfileplacesview.cpp:169
> I think in to move away from workarounds. Cause we want to draw header only ones i you ask to check a QStyleOptionViewItem index if you can use it insetad of dragcount.

@anthonyfieroni Well, at paint time you need to determine if a drag is ongoing or not. There's no way the index can tell you that.

REPOSITORY
  R241 KIO

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

To: renatoo, #frameworks, dfaure, ervin
Cc: mlaurent, ervin, anthonyfieroni, cfeck, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20171013/636b59eb/attachment.html>