D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  In D22617#500212 , @ndavis wrote:
  
  > In D22617#500031 , @davidhurka 
wrote:
  >
  > > I can’t follow you here. Centering the first page is a feature / 
technical detail of Okular, but it indicates that the first page is somehow 
special, like the cover page of a book. But your document could be a single 
chapter of the book as well, so the first page is just a regular odd-numbered 
page. That page should be aligned right.
  >
  >
  > Since it's an icon for Okular and the feature is called "Facing Pages 
(Center First Page), I don't understand what the problem is with making the top 
symbol centered.
  >
  > Here's what I had in mind:
  >  F7077350: view-pages-facing-first-centered.svg 
 F7077351: 
view-pages-facing-first-centered.svg 
  
  
  Just thinking about RTL layout, centering the top symbol is probably better. 
Fold will still be on the right, because all the other icons already have it on 
the right...

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Noah Davis]

ndavis added a comment.


  In D22617#500168 , @davidhurka 
wrote:
  
  > Other icons with fold in the bottom I could find:
  >
  > - document-duplicate
  > - document-revert-symbolic[-rtl]
  > - kt-restore-defaults
  > - password-copy
  > - viewpdf
  > - xml-node-duplicate
  > - user-desktop-symbolic
  > - document-multiple
  > - folder-documents[-symbolic]
  > - emblem-documents-symbolic
  >
  >   Most icons indeed have the fold at the top, especially Mimetypes icons 
look great. :)
  
  
  Thanks! In case you're not sure, you don't need to change any of those in 
this patch.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Noah Davis]

ndavis added a comment.


  In D22617#500031 , @davidhurka 
wrote:
  
  > Makes sense, so I’m flipping snap-page now. Is that written down somewhere?
  
  
  Nope, it was recently decided, but I've had that in the back of my mind for a 
while.
  
  > Previously I thought a fold on the top is for documents, and a fold on the 
bottom for pages. Icons like file-new have a fold on the top, icons like 
insert-page-break have it at the bottom.
  
  I think it doesn't have anything to do with whether the icon is for pages or 
documents, it's just that the document-new icon was created more recently with 
a different style. It might be useful to have a distinction between pages and 
documents, but a different position for the fold isn't enough and I'm not sure 
how to make the difference obvious.
  
  >> Naming Scheme:
  >>  [...]
  > 
  > Because it’s changing the layout of page//s//, not a layout //in a// page, 
yes?
  
  Mainly just to be consistent with existing icon naming schemes, but that's 
another possible reason that I didn't think of.
  
  >> `pagelayout-facing`:
  >>  [...]
  > 
  > I have increased the height by 1px for the 22px icon.
  
  It still looks pretty squished. Here's what I was thinking of: F7077302: 
view-pages-facing.svg 
  
  >> `pagelayout-facing-first-centered`: 
  >>  [...]
  > 
  > I can’t follow you here. Centering the first page is a feature / technical 
detail of Okular, but it indicates that the first page is somehow special, like 
the cover page of a book. But your document could be a single chapter of the 
book as well, so the first page is just a regular odd-numbered page. That page 
should be aligned right.
  
  Since it's an icon for Okular and the feature is called "Facing Pages (Center 
First Page), I don't understand what the problem is with making the top symbol 
centered.
  
  Here's what I had in mind:
  F7077350: view-pages-facing-first-centered.svg 
 F7077351: 
view-pages-facing-first-centered.svg 
  
  In D22617#500141 , @davidhurka 
wrote:
  
  > But it is just a redundancy. Can’t there be a script, which copies icons to 
icons-dark and changes the colors, when compiling the icons?
  
  
  For most icons, probably.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Nathaniel Graham]

ngraham added a comment.


  In D22617#500141 , @davidhurka 
wrote:
  
  > In D22617#500131 , @ndavis wrote:
  >
  > > I'll give the latest changes a proper review in a little while.
  > >
  > > In D22617#500046 , @davidhurka 
wrote:
  > >
  > > > What is the icons-dark directory good for? The icons in there are 
mostly just the same as in icons.
  > >
  > >
  > > Non-Qt apps, since they don't work with our stylesheets. Hopefully an XDG 
standard for coloring symbolic/monochrome icons will fix that and then we can 
drop icons-dark.
  >
  >
  > But it is just a redundancy. Can’t there be a script, which copies icons to 
icons-dark and changes the colors, when compiling the icons?
  
  
  Oh that sounds like a lovely idea!!!

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  In D22617#499767 , @ndavis wrote:
  
  > Nice work!
  >
  > I know a lot of monochrome icons currently use the bottom right position 
for the folded corner, but I think we should start using the top right. The 
bottom right clashes with our convention of putting additional symbols in the 
bottom right and all of our color icons use the top right for the corner fold.
  
  
  Other icons with fold in the bottom I could find:
  
  - document-duplicate
  - document-revert-symbolic[-rtl]
  - kt-restore-defaults
  - password-copy
  - viewpdf
  - xml-node-duplicate
  - user-desktop-symbolic
  - document-multiple
  - folder-documents[-symbolic]
  - emblem-documents-symbolic
  
  Most icons indeed have the fold at the top, especially Mimetypes icons look 
great. :)

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  In D22617#500131 , @ndavis wrote:
  
  > I'll give the latest changes a proper review in a little while.
  >
  > In D22617#500046 , @davidhurka 
wrote:
  >
  > > What is the icons-dark directory good for? The icons in there are mostly 
just the same as in icons.
  >
  >
  > Non-Qt apps, since they don't work with our stylesheets. Hopefully an XDG 
standard for coloring symbolic/monochrome icons will fix that and then we can 
drop icons-dark.
  
  
  But it is just a redundancy. Can’t there be a script, which copies icons to 
icons-dark and changes the colors, when compiling the icons?
  
  >> And I’m a bit concerned that scour-icon removes the 
id="currentColorScheme" attribute from the stylesheet. How do I avoid that?
  > 
  > Hm, you could try removing `--enable-id-stripping` and `--shorten-ids` from 
the command. If anything from this page 
 confuses 
you or doesn't work, be sure to post in the Discussion section. Writing 
documentation isn't a strength of mine, but it's something that has to be done.
  
  Done.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Noah Davis]

ndavis added a comment.


  I'll give the latest changes a proper review in a little while.
  
  In D22617#500046 , @davidhurka 
wrote:
  
  > What is the icons-dark directory good for? The icons in there are mostly 
just the same as in icons.
  
  
  Non-Qt apps, since they don't work with our stylesheets. Hopefully an XDG 
standard for coloring symbolic/monochrome icons will fix that and then we can 
drop icons-dark.
  
  > And I’m a bit concerned that scour-icon removes the id="currentColorScheme" 
attribute from the stylesheet. How do I avoid that?
  
  Hm, you could try removing `--enable-id-stripping` and `--shorten-ids` from 
the command. If anything from this page 
 confuses 
you or doesn't work, be sure to post in the Discussion section. Writing 
documentation isn't a strength of mine, but it's something that has to be done.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  What is the icons-dark directory good for? The icons in there are mostly just 
the same as in icons.
  
  And I’m a bit concerned that scour-icon removes the id="currentColorScheme" 
attribute from the stylesheet. How do I avoid that?

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka updated this revision to Diff 62294.
davidhurka added a comment.


  - Rename pagelayout-* to view-pages-*

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62293&id=62294

BRANCH
  add-icons-for-pagelayouts

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

AFFECTED FILES
  icons-dark/actions/16/snap-page.svg
  icons-dark/actions/16/view-pages-continuous.svg
  icons-dark/actions/16/view-pages-facing-first-centered.svg
  icons-dark/actions/16/view-pages-facing.svg
  icons-dark/actions/16/view-pages-overview.svg
  icons-dark/actions/16/view-pages-single.svg
  icons-dark/actions/22/snap-page.svg
  icons-dark/actions/22/view-pages-continuous.svg
  icons-dark/actions/22/view-pages-facing-first-centered.svg
  icons-dark/actions/22/view-pages-facing.svg
  icons-dark/actions/22/view-pages-overview.svg
  icons-dark/actions/22/view-pages-single.svg
  icons-dark/actions/24/snap-page.svg
  icons/actions/16/snap-page.svg
  icons/actions/16/view-pages-continuous.svg
  icons/actions/16/view-pages-facing-first-centered.svg
  icons/actions/16/view-pages-facing.svg
  icons/actions/16/view-pages-overview.svg
  icons/actions/16/view-pages-single.svg
  icons/actions/22/snap-page.svg
  icons/actions/22/view-pages-continuous.svg
  icons/actions/22/view-pages-facing-first-centered.svg
  icons/actions/22/view-pages-facing.svg
  icons/actions/22/view-pages-overview.svg
  icons/actions/22/view-pages-single.svg
  icons/actions/24/snap-page.svg

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka updated this revision to Diff 62293.
davidhurka added a comment.


  - Remove 24px icons
  - Flip snap-page icons vertically, so the corner fold is at the top-right
  - Make pagelayout-single link to snap-page
  - Flip pagelayout-* icons, except pagelayout-single

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62208&id=62293

BRANCH
  add-icons-for-pagelayouts

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

AFFECTED FILES
  icons-dark/actions/16/pagelayout-continuous.svg
  icons-dark/actions/16/pagelayout-facing-first-centered.svg
  icons-dark/actions/16/pagelayout-facing.svg
  icons-dark/actions/16/pagelayout-overview.svg
  icons-dark/actions/16/pagelayout-single.svg
  icons-dark/actions/16/snap-page.svg
  icons-dark/actions/22/pagelayout-continuous.svg
  icons-dark/actions/22/pagelayout-facing-first-centered.svg
  icons-dark/actions/22/pagelayout-facing.svg
  icons-dark/actions/22/pagelayout-overview.svg
  icons-dark/actions/22/pagelayout-single.svg
  icons-dark/actions/22/snap-page.svg
  icons-dark/actions/24/snap-page.svg
  icons/actions/16/pagelayout-continuous.svg
  icons/actions/16/pagelayout-facing-first-centered.svg
  icons/actions/16/pagelayout-facing.svg
  icons/actions/16/pagelayout-overview.svg
  icons/actions/16/pagelayout-single.svg
  icons/actions/16/snap-page.svg
  icons/actions/22/pagelayout-continuous.svg
  icons/actions/22/pagelayout-facing-first-centered.svg
  icons/actions/22/pagelayout-facing.svg
  icons/actions/22/pagelayout-overview.svg
  icons/actions/22/pagelayout-single.svg
  icons/actions/22/snap-page.svg
  icons/actions/24/snap-page.svg

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  
  
  In D22617#499767 , @ndavis wrote:
  
  > Nice work!
  >
  > I know a lot of monochrome icons currently use the bottom right position 
for the folded corner, but I think we should start using the top right. The 
bottom right clashes with our convention of putting additional symbols in the 
bottom right and all of our color icons use the top right for the corner fold.
  
  
  Makes sense, so I’m flipping snap-page now. Is that written down somewhere?
  
  Previously I thought a fold on the top is for documents, and a fold on the 
bottom for pages. Icons like file-new have a fold on the top, icons like 
insert-page-break have it at the bottom.
  
  > There are some additional changes I would like to see.
  > 
  > Naming scheme:
  >  Since this is changing the view, the naming scheme should be something 
like `view-pages-*` rather than `pagelayout-*`
  
  Because it’s changing the layout of page//s//, not a layout //in a// page, 
yes?
  
  > `pagelayout-single`: 
  >  This should be a relative symlink to `snap-page.svg` since they're the 
same icon.
  
  Ok.
  
  > `pagelayout-facing`:
  >  The 16px version is good, but the larger versions don't use up enough of 
the available canvas. In general, aim to use 100% of the allowed vertical space 
(16px in height with 3px top/bottom margins for 22px) unless that would force 
you to squish the symbols in an unattractive way.
  
  I have increased the height by 1px for the 22px icon.
  
  > `pagelayout-facing-first-centered`: 
  >  The top page should be centered to reflect how the view mode works. I 
think the top page shouldn't be cut off, but that means you'll need to shrink 
the size of the pages overall. This will be particularly difficult for the 16px 
version. If you can't get the 16px version to look right with a 2px margin on 
the top and bottom, I'll accept it with a 1px margin even though that goes 
against the HIG.
  
  I can’t follow you here. Centering the first page is a feature / technical 
detail of Okular, but it indicates that the first page is somehow special, like 
the cover page of a book. But your document could be a single chapter of the 
book as well, so the first page is just a regular odd-numbered page. That page 
should be aligned right.
  
  Now my 16px icon looks like a bunch of arrows, not so fine...
  
  >  ---
  > 
  > I don't think it's necessary to do 24px versions of these icons, but 
there's no harm in doing them. It's just more work.
  
  Removed them.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Noah Davis]

ndavis requested changes to this revision.
ndavis added a comment.
This revision now requires changes to proceed.


  Nice work!
  
  I know a lot of monochrome icons currently use the bottom right position for 
the folded corner, but I think we should start using the top right. The bottom 
right clashes with our convention of putting additional symbols in the bottom 
right and all of our color icons use the top right for the corner fold.
  
  There are some additional changes I would like to see.
  
  Naming scheme:
  Since this is changing the view, the naming scheme should be something like 
`view-pages-*` rather than `pagelayout-*`
  
  `pagelayout-single`: 
  This should be a symlink to `snap-page.svg` since they're the same icon.
  
  `pagelayout-facing`:
  The 16px version is good, but the larger versions don't use up enough of the 
available canvas. In general, aim to use 100% of the allowed vertical space 
(16px in height with 3px top/bottom margins for 22px) unless that would force 
you to squish the symbols in an unattractive way.
  
  `pagelayout-facing-first-centered`: 
  The top page should be centered to reflect how the view mode works. I think 
the top page shouldn't be cut off, but that means you'll need to shrink the 
size of the pages overall. This will be particularly difficult for the 16px 
version. If you can't get the 16px version to look right with a 2px margin on 
the top and bottom, I'll accept it with a 1px margin even though that goes 
against the HIG.
  
  `pagelayout-overview`: 
  It should represent pages laid out in a 2x2 grid rather than a stack since 
Overview mode has a grid layout.
  
  ---
  
  I don't think it's necessary to do 24px versions of these icons, but there's 
no harm in doing them. It's just more work.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[Nathaniel Graham]

ngraham added a reviewer: ndavis.
ngraham added a comment.


  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 T10165 ). When in doubt, if it's a 
monochrome action icon, you can safely make 16px and 22px versions and call it 
a day.
  
  This definitely needs some cleanup in the documentation, for sure.
  
  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.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg, ndavis
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a subscriber: Okular.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka added a comment.


  I’m not sure whether I did everything correctly. If I did, this is too 
complicated.
  
  The guidelines in the community wiki 
 and the 
HIG  are missing:
  
  - What are the borders good for?
  - What icon sizes are needed? (I choosed 16, 22, 24, because theese have the 
most icons.)
  - What is 24px good for? Isn’t it just 22px with 1px additional border?
  
  And there are just workflow tips, is the actual workflow for adding icons 
described somewhere?
  
  One more problem: scour breaks up the colors of the path elements, if they 
are more than 2. It moves the fill attribute in a group, but then the color 
attributes seem to override it, so the stylesheet is ignored.

REPOSITORY
  R266 Breeze Icons

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

To: davidhurka, #vdg
Cc: kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, bruns

D22617: Add icons for pagelayout options, as needed in Okular

[David Hurka]

davidhurka created this revision.
davidhurka added a reviewer: VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidhurka requested review of this revision.

REVISION SUMMARY
  This is my try to add the icons I requested in D21196#484674 
.
  
  The icons are: pagelayout-[single|facing|facing-first-centered|overview], as 
they are needed for the View Layout menu; and pagelayout-continuous, as it is 
needed for the Continuous view option (which might be moved into the View 
Layout menu).
  
  BUG: 409082

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-icons-for-pagelayouts

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

AFFECTED FILES
  icons-dark/actions/16/pagelayout-continuous.svg
  icons-dark/actions/16/pagelayout-facing-first-centered.svg
  icons-dark/actions/16/pagelayout-facing.svg
  icons-dark/actions/16/pagelayout-overview.svg
  icons-dark/actions/16/pagelayout-single.svg
  icons-dark/actions/22/pagelayout-continuous.svg
  icons-dark/actions/22/pagelayout-facing-first-centered.svg
  icons-dark/actions/22/pagelayout-facing.svg
  icons-dark/actions/22/pagelayout-overview.svg
  icons-dark/actions/22/pagelayout-single.svg
  icons-dark/actions/24/pagelayout-continuous.svg
  icons-dark/actions/24/pagelayout-facing-first-centered.svg
  icons-dark/actions/24/pagelayout-facing.svg
  icons-dark/actions/24/pagelayout-overview.svg
  icons-dark/actions/24/pagelayout-single.svg
  icons/actions/16/pagelayout-continuous.svg
  icons/actions/16/pagelayout-facing-first-centered.svg
  icons/actions/16/pagelayout-facing.svg
  icons/actions/16/pagelayout-overview.svg
  icons/actions/16/pagelayout-single.svg
  icons/actions/22/pagelayout-continuous.svg
  icons/actions/22/pagelayout-facing-first-centered.svg
  icons/actions/22/pagelayout-facing.svg
  icons/actions/22/pagelayout-overview.svg
  icons/actions/22/pagelayout-single.svg
  icons/actions/24/pagelayout-continuous.svg
  icons/actions/24/pagelayout-facing-first-centered.svg
  icons/actions/24/pagelayout-facing.svg
  icons/actions/24/pagelayout-overview.svg
  icons/actions/24/pagelayout-single.svg

To: davidhurka, #vdg
Cc: kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, bruns