ngraham added subscribers: bcooksley, ngraham. ngraham added a comment.
Wow, these are truly excellent. I think you've done an amazing job! One thing I'd like to discuss is whether or not we want the `emblem-remove` icon to be red. This color is typically reserved for destructive actions and error conditions, and the emblem as far as I can tell is only used in Dolphin--where its usage denotes something that is neither destructive nor an error. Users might be worried that clicking on it will actually remove the item! I wonder if a the Icon Orange color might be more suitable. What do you think? Another thing is the `emblem-symbolic-link` icon. It's the only common-ish one that doesn't follow the pattern of having a colored background with a border, which might muddy the design language you've chosen (which I love). Also, I don't think the filled-in background really works: F6349078: link icon.png <https://phabricator.kde.org/F6349078> Just throwing out some discussion points, but those are pretty minor and overall this is already a big improvement IMHO. --- Since this fixes all three bugs, you can replace https://bugs.kde.org/show_bug.cgi?id=399356 https://bugs.kde.org/show_bug.cgi?id=399357 https://bugs.kde.org/show_bug.cgi?id=399968 with BUG: 399356 BUG: 399357 BUG: 399968 FIXED-IN: 5.52 --- Unfortunately, the patch does not apply cleanly, and I don't think it's your fault: arc patch D16421 [...] Checking patch icons/emblems/22/emblem-pause.svg... Checking patch icons/emblems/22/emblem-mounted.svg... Checking patch dev/null => icons/emblems/22/emblem-locked.svg... error: dev/null: does not exist in index Checking patch icons/emblems/22/emblem-information.svg... Checking patch icons/emblems/22/emblem-important.svg... Checking patch dev/null => icons/emblems/22/emblem-favorite.svg... error: dev/null: does not exist in index Checking patch icons/emblems/22/emblem-error.svg... Checking patch icons/emblems/22/emblem-encrypted-unlocked.svg... [...] Checking patch icons-dark/emblems/22/emblem-mounted.svg... Checking patch dev/null => icons-dark/emblems/22/emblem-locked.svg... error: dev/null: does not exist in index Checking patch icons-dark/emblems/22/emblem-information.svg... Checking patch icons-dark/emblems/22/emblem-important.svg... Checking patch dev/null => icons-dark/emblems/22/emblem-favorite.svg... error: dev/null: does not exist in index Checking patch icons-dark/emblems/22/emblem-error.svg... Checking patch icons-dark/emblems/22/emblem-encrypted-unlocked.svg... What's going on here is that some symlinks are being replaced with new files, and other files are being replaced with symlinks. `arc` doesn't seem too happy about this. @bcooksley or anyone else from #sysadmin <https://phabricator.kde.org/tag/sysadmin/>, any idea what to do here? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D16421 To: ndavis, #vdg Cc: ngraham, bcooksley, kde-frameworks-devel, #vdg, michaelh, bruns
