<table><tr><td style="">ngraham added a reviewer: ndavis.<br />ngraham added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D22617">View Revision</a></tr></table><br /><div><div><p>The 16px version is used in menus. The 22px version is used in toolbar buttons. 24px is unnecessary; it's only used by some 3rd-party apps I think. The ones that are already in there at that size are legacy and maybe we should just delete them? 32px is used for category icons and should be colorized. In general there isn't a reason to create monochrome 24x or 32px icons (and the few places where we are actually using 32px monochrome icons should be changed IMO; see <a href="https://phabricator.kde.org/T10165" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">T10165</a>). When in doubt, if it's a monochrome action icon, you can safely make 16px and 22px versions and call it a day.</p>
<p>This definitely needs some cleanup in the documentation, for sure.</p>
<p>In terms of the icons themselves, they look good! I have a visual change request though: could you put the page curl on the top rather than on the bottom. That's the general style that most Breeze document icons use, and it would be good to use that. Basically just vertically flipping the icons should be sufficient.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R266 Breeze Icons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22617">https://phabricator.kde.org/D22617</a></div></div><br /><div><strong>To: </strong>davidhurka, VDG, ndavis<br /><strong>Cc: </strong>Okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, bruns<br /></div>