D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:7069d6f0cb3d: Add view-pages-* icons, as needed in Okular 
for page layout selection (authored by davidhurka, committed by ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62603=62608

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, ngraham
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[Nathaniel Graham]

ngraham added a comment.


  Thanks very much! So now we can expect to see an Okular patch, right? :)

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-icons-for-pagelayouts

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

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

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[Noah Davis]

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


  Looks ready to land!
  
  In D22617#502629 , @davidhurka 
wrote:
  
  > - Add missing .ColorScheme-Text { to 24px versions of snap-page.svg
  >
  >   And then the class name was missing, together with the opening brace: 
`.ColorScheme-Text {`. This is not a comfortable workflow...
  
  
  It could be a lot better, I've just gotten used to it and the confusion from 
learning it for the first time is gone. Ideally, we'd have a CSS file that all 
the SVGs link to instead of embedding stylesheets, or maybe we'd do something 
else that I don't know about. I tried doing the former and couldn't figure out 
how to make it work.
  
  > Would it make sense to parse the SVG files and collect the path elements in 
QPainterPaths, to put them in a new SVG file with the correct color scheme CSS 
stuff?
  
  TBH, I don't know what you're talking about. I don't actually know Qt that 
well.

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-icons-for-pagelayouts

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

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

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

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


  - Add missing .ColorScheme-Text { to 24px versions of snap-page.svg
  
  And then the class name was missing, together with the opening brace: 
`.ColorScheme-Text {`. This is not a comfortable workflow...
  
  Would it make sense to parse the SVG files and collect the path elements in 
QPainterPaths, to put them in a new SVG file with the correct color scheme CSS 
stuff?

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62602=62603

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, ngraham
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka marked 2 inline comments as done.

REPOSITORY
  R266 Breeze Icons

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

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

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

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


  - Add missing id=currentColorScheme to 24px versions of snap-page
  
  These snap-page.svg files weren’t in 16px nor 22px, so I forgot them when I 
replaced `type="text/css"` with `type="text/css" id="currentColorScheme"` in 
all files. Thanks for watching out.
  
  > The current color is determined by the color class.
  
  So `currentColor` is a property of any CSS class?

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62415=62602

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, ngraham
Cc: #okular, kde-frameworks-devel, ngraham, LeGast00n, sbergeron, michaelh, 
bruns

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[Noah Davis]

ndavis added a comment.


  In D22617#500953 , @davidhurka 
wrote:
  
  > What does `fill:currentColor` mean, by the way?
  
  
  The current color is determined by the color class. There has to be a `fill`, 
so we set it to `currentColor`.

INLINE COMMENTS

> snap-page.svg:3
> +
> +

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  LGTM now. Very nice work.
  
  @ndavis, are you happy too?

REPOSITORY
  R266 Breeze Icons

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

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

D22617: Add view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka added a comment.


  What does `fill:currentColor` mean, by the way?

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

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


  - Remove color attributes, which were added by scour-icon

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62411=62415

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka added a comment.


  I assume that `fill="currentColor"` also doesn’t work as expected, fixing 
that now...

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

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


  - Fix id=current-color-scheme attributes which were removed by scour-icon.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22617?vs=62332=62411

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 view-pages-* icons, as needed in Okular for page layout selection

[Noah Davis]

ndavis added a comment.


  In D22617#500369 , @davidhurka 
wrote:
  
  > Making the parts with background color transparent would be better, right? 
That would even work on systems which don’t access the stylesheet.
  
  
  Not necessarily. In Breeze, the normal window Background is `#eff0f1`, the 
ViewBackground is `#fcfcfc` and the selection background is `#3daee9`. If you 
use the Background class, you will have `#eff0f1`, whether the SVG is on top of 
a normal window background or not. This is more useful for Plasma themes than 
icons, but they're both made with SVGs and can use the same color classes.
  
  > So this means, changing the Neutral Text property on your system will 
modify the Beware Orange parts of breeze icons, yes? That’s what I meant with 
mapping.
  > 
  > I could imagine adding this to the HIG: A table with the reduced breeze 
color palette, the color names, the corresponding color scheme entries, and the 
existing description to the color. (So only add the color scheme entry.)
  
  Yeah, showing a clearer relationship between color classes and default colors 
would be good.
  
  >> or fork, patch and merge request to submit a patch for that repo.
  > 
  > Not today, have to leave the wall plug now... :)
  
  Understandable.

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka added a comment.


  In D22617#500361 , @ndavis wrote:
  
  > In D22617#500346 , @davidhurka 
wrote:
  >
  > > - Why does the stylesheet need a background?
  >
  >
  > You only need to add the classes that you will use. If you want to use the 
window background color, you can use the Background class.
  
  
  Making the parts with background color transparent would be better, right? 
That would even work on systems which don’t access the stylesheet.
  
  >> - How are the color scheme properties in the stylesheet mapped to the 
reduced breeze palette in the HIG? (I think neutral or Beware Orange do not 
clearly map.)
  > 
  > Beware Orange is just the name of a color (it's an arbitrary name). 
NeutralText is a class that maps to the Neutral Text color in your system 
colorscheme. The color of Neutral Text just happens to be Beware Orange with 
the Breeze colorscheme, but you could make it purple by changing the 
colorscheme. Since Breeze uses Beware Orange, that color must be the default 
color in the stylesheet so that the icon color looks like Breeze in non-Qt apps.
  
  So this means, changing the Neutral Text property on your system will modify 
the Beware Orange parts of breeze icons, yes? That’s what I meant with mapping.
  
  I could imagine adding this to the HIG: A table with the reduced breeze color 
palette, the color names, the corresponding color scheme entries, and the 
existing description to the color. (So only add the color scheme entry.)
  
  > If you find any more flaws in the HIG, report an issue at 
https://invent.kde.org/websites/hig-kde-org/issues
  
  Concerning margins: https://invent.kde.org/websites/hig-kde-org/issues/12
  
  > or fork, patch and merge request to submit a patch for that repo.
  
  Not today, have to leave the wall plug now... :)

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 view-pages-* icons, as needed in Okular for page layout selection

[Noah Davis]

ndavis added a comment.


  In D22617#500346 , @davidhurka 
wrote:
  
  > By the way, the suggested stylesheet in 
https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Breeze does 
not follow https://hig.kde.org/style/icon.html as far as I can understand it.
  
  
  I just changed the stylesheet in the workflow tips today. I need to update 
the HIG to match it.
  
  > - Why does the stylesheet need a background?
  
  You only need to add the classes that you will use. If you want to use the 
window background color, you can use the Background class.
  
  > - How are the color scheme properties in the stylesheet mapped to the 
reduced breeze palette in the HIG? (I think neutral or Beware Orange do not 
clearly map.)
  
  Beware Orange is just the name of a color (it's an arbitrary name). 
NeutralText is a class that maps to the Neutral Text color in your system 
colorscheme. The color of Neutral Text just happens to be Beware Orange with 
the Breeze colorscheme, but you could make it purple by changing the 
colorscheme. Since Breeze uses Beware Orange, that color must be the default 
color in the stylesheet so that the icon color looks like Breeze in non-Qt apps.
  
  If you find any more flaws in the HIG, report an issue at 
https://invent.kde.org/websites/hig-kde-org/issues or fork, patch and merge 
request to submit a patch for that repo.

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 view-pages-* icons, as needed in Okular for page layout selection

[Nathaniel Graham]

ngraham added a comment.


  These versions look great to me now. :)

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka added a comment.


  By the way, the suggested stylesheet in 
https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips#Breeze does 
not follow https://hig.kde.org/style/icon.html as far as I can understand it.
  
  - Why does the stylesheet need a background?
  - How are the color scheme properties in the stylesheet mapped to the reduced 
breeze palette in the HIG? (I think neutral or Beware Orange do not clearly 
map.)
  
  If this is ok now, I still have to add the lost `id="current-color-scheme"`.

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

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


  - Adapt view-pages-facing and -facing-first-centered to use ndavis' suggested 
files

REPOSITORY
  R266 Breeze Icons

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

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 view-pages-* icons, as needed in Okular for page layout selection

[David Hurka]

davidhurka retitled this revision from "Add icons for pagelayout options, as 
needed in Okular" to "Add view-pages-* icons, as needed in Okular for page 
layout selection".
davidhurka edited the summary of 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