D16019: Added missing icons to panel places items

Nathaniel Graham noreply at phabricator.kde.org
Sun Oct 7 21:32:27 BST 2018


ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  In D16019#338462 <https://phabricator.kde.org/D16019#338462>, @acrouthamel wrote:
  
  > Also, it may not be best to choose a Breeze icon based on look, rather than name. Other icon themes may use something entirely different for hint.
  
  
  For the moment this is probably the best we can do. Dolphin uses the same `visibility`/`hint` switcheroo for the Hidden Files action which I agree is not ideal. I've been mildly irritated by this myself when browsing the code, but never enough to do anything about it. I just tried submitting a patch to change it (D16028 <https://phabricator.kde.org/D16028>) but ran into `arc` problems and gave up.
  
  For now, please stay with `visibility` and `hint`, this is fine.
  
  Typically it's not ideal to use the same icon for two different adjacent things, but I think it's fine for now, and better than no icon.
  
  This looks good! But there's one more thing...
  
  Dolphin currently does not use the KIO code for parts of its places panel, so this change needs to be replicated for the KIO code so that Places panels in the open/save dialogs and Gwenview (etc.) get the change too. Can you submit a patch for KIO too? The code is in `<kio repo>/src/panels/places/placespanel.cpp` Thanks! Once you submit that patch and both are accepted, we can land them both.

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

To: alexde, #vdg, ngraham
Cc: ngraham, acrouthamel, kfm-devel, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20181007/68b17652/attachment.htm>


More information about the kfm-devel mailing list