D22653: Add view-barcode-qr icons

This revision was automatically updated to reflect the committed changes. Closed by commit R266:7461f90af829: Add view-barcode-qr icons (authored by ndavis). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=64574&id=64575 REVISION DETAIL https:/

D22653: Add view-barcode-qr icons

ndavis marked 6 inline comments as done. REPOSITORY R266 Breeze Icons BRANCH arcpatch-D22653 REVISION DETAIL https://phabricator.kde.org/D22653 To: ndavis, #breeze, ngraham, mbruchert Cc: lavender, ngraham, broulik, ndavis, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D22653: Add view-barcode-qr icons

ndavis updated this revision to Diff 64574. ndavis added a comment. - Make 32px be 32px REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62915&id=64574 BRANCH arcpatch-D22653 REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FIL

D22653: Add view-barcode-qr icons

ndavis commandeered this revision. ndavis edited reviewers, added: mbruchert; removed: ndavis. This revision is now accepted and ready to land. REPOSITORY R266 Breeze Icons BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 To: ndavis, #breeze, ngraham, mbruchert Cc: lavende

D22653: Add view-barcode-qr icons

ndavis added a comment. The 32px icons need to be 32x32, not 54x54. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D22653 To: mbruchert, #breeze, ndavis, ngraham Cc: lavender, ngraham, broulik, ndavis, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, bruns

D22653: Add view-barcode-qr icons

lavender added a comment. In D22653#505659 , @ndavis wrote: > I try not to be too aggressive with optimization when it comes to diffs from other people because the process of making an icon is currently quite tedious and the optimizations

D22653: Add view-barcode-qr icons

ndavis added a comment. actually, you can disregard my inline comments on the 32px icons, I didn't realize you had already removed the duplicate fill currentColor bits. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D22653 To: mbruchert, #breeze, ndavis, ngraha

D22653: Add view-barcode-qr icons

ndavis added inline comments. INLINE COMMENTS > view-barcode-qr.svg:7 > + > + style="fill:currentColor;fill-opacity:1;stroke:none;stroke-width:2" d="M 2 2 > L 2 6 L 6 6 L 6 2 L 2 2 z M 8 2 L 8 3 L 9 3 L 9 2 L 8 2 z M 8 3 L 7 3 L 7 5 L > 8 5 L 8 3 z M 8 5 L 8 6 L 9 6 L 9 5 L 8 5 z M 8 6

D22653: Add view-barcode-qr icons

ngraham accepted this revision. ngraham added a comment. LGTM! Let's wait for @ndavis's full review before landing thiw REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D22653 To: mbruchert, #breeze, ndavis, ngraham Cc: lavender, ngraham, broulik, ndavis, kde-fram

D22653: Add view-barcode-qr icons

ndavis added a comment. In D22653#505439 , @lavender wrote: > [snip] > And the 32 one has a transform that can be applied: > > http://www.w3.org/2000/svg"; id="svg938" width="54" height="54">.ColorScheme-Text{color:#232629} > > > I

D22653: Add view-barcode-qr icons

lavender added a comment. Functionally it looks good to me, there are a few improvements/optimizations that can be made for example 16 and 22 have unnecessary attributes such as: fill="currentColor" style="fill:currentColor; And the 32 one has a transform that can be applied:

D22653: Add view-barcode-qr icons

mbruchert updated this revision to Diff 62915. mbruchert added a comment. remove paint-order REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62913&id=62915 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FILES

D22653: Add view-barcode-qr icons

mbruchert updated this revision to Diff 62913. mbruchert added a comment. - remove corners REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62761&id=62913 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FILES i

D22653: Add view-barcode-qr icons

ndavis added a comment. In D22653#504138 , @lavender wrote: > I noticed that only the 22px icons use the viewbox, is this intentional? > > As for the 32px one the validator I used complains that: > > Error: Attri

D22653: Add view-barcode-qr icons

lavender added a comment. I noticed that only the 22px icons use the viewbox, is this intentional? The validator I used complains that: Error: Attribute paint-order not allowed on SVG element path at this point. From line 7, column 5; to line 7,

D22653: Add view-barcode-qr icons

ndavis added a comment. I think you accidentally included a bunch of unrelated files. BTW, I'm currently on vacation and can't access my laptop. Could you show me a screenshot of the 32px icon at 1x size? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D22653 T

D22653: Add view-barcode-qr icons

mbruchert updated this revision to Diff 62761. mbruchert added a comment. remove corners REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62759&id=62761 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FILES ico

D22653: Add view-barcode-qr icons

mbruchert updated this revision to Diff 62759. mbruchert added a comment. remove corners REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62754&id=62759 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FILES ico

D22653: Add view-barcode-qr icons

mbruchert updated this revision to Diff 62754. mbruchert added a comment. - remove blue corners REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22653?vs=62460&id=62754 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22653 AFFECTED FILE

D22653: Add view-barcode-qr icons

ngraham added a comment. In D22653#501534 , @ndavis wrote: > I think I do want the 32px version to not have the blue corners so that it is visually consistent with the smaller versions. Once that is done, I will accept this, as long as there are

D22653: Add view-barcode-qr icons

ndavis added a comment. I think I do want the 32px version to not have the blue corners so that it is visually consistent with the smaller versions. Once that is done, I will accept this, as long as there aren't any issues with the colorscheme support. REPOSITORY R266 Breeze Icons REVISIO

D22653: Add view-barcode-qr icons

mbruchert retitled this revision from "Add view-qrcode icons" to "Add view-barcode-qr icons". REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D22653 To: mbruchert, #breeze, ndavis Cc: broulik, ndavis, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, b