D16365: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat added a comment.


  Thanks for the comments, review and for splitting up the diff. I also know it 
was daunting to review the patch in its original form and I could've done that 
better.

REPOSITORY
  R98 Breeze for Gtk

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

To: ohelin, jackg, #breeze, #plasma, ngraham
Cc: grmat, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15786: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat added a comment.


  Thanks for the review, comments and for splitting up the diff. I also know it 
was daunting to review the patch in its original form and I could've done that 
better.
  I'll see if I can test the updated version in the upcoming days.

REPOSITORY
  R98 Breeze for Gtk

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

To: grmat, jackg, #plasma
Cc: ngraham, ohelin, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D14068: add -P switch to one-shot redshift call

[mat gr]

grmat added a comment.


  In D14068#332672 , @davidedmundson 
wrote:
  
  > what happens to users on older versions?
  
  
  It breaks, as redshift <1.12 will exit with "invalid option -- 'P'".
  
  I initially just thought of this as packaging responsibility, however, users 
might install those applets independently from distribution's packages.
  
  It would be possible to make a runtime check, look for the version no. with 
`redshift -V` and add the -P switch dynamically. However, I'm not familiar with 
QML or where this call would be added. I thought of something like
  
...
property string redshiftCompat: getInstalledRedshiftVersion().split(' ')[1] 
< '1.12' ? '' : '-P'
property string redshiftOneTimeCommand: 'redshift -O ' + redshiftCompat + 
...

REPOSITORY
  R885 Redshift Control for Plasma

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

To: grmat, #plasma, romangg, martinkostolny
Cc: davidedmundson, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15786: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat added a comment.


  additional note: commit 1ea1e0f88d0812d0f7b8e542bd9eacee234a93ee 
 is 
also strongly related to this change as it introduced lots of additional colour 
variables. For this change, the dark scheme was skipped as well. So I've also 
looked at that tried to match the respective colours by looking at previous 
revisions of both variants.

REPOSITORY
  R98 Breeze for Gtk

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

To: grmat, jackg, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15786: share common values for both Breeze and Breeze-dark GTK themes

[mat gr]

grmat created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
grmat requested review of this revision.

REVISION SUMMARY
  As described in bug #396091, the Breeze-dark theme is often neglected in 
updates. This patch fixes #396091 and some additional, related inconsistencies, 
enables sharing the basic stuff like shape, size, style, etc. of components in 
a common single css file while keeping the colour definitions separated from 
that in different files for both the light and dark theme.
  This allows easier changes, easier maintenance because of less LOC, less 
duplicate code as well as easier extensibility for potential additional colour 
schemes like light  or 
high-contrast .
  
  I only did this for GTK 3.20 for now. If you like the effort, it could (and 
maybe should) get extended to other versions.
  
  Further potential steps in the same direction of saving code would be going 
back to SASS, which is used by many other popular GTK themes and was used by 
Breeze-gtk as well in the past. The only downside of that is that the SASS 
source files would have to be "compiled" to CSS prior to packaging.

TEST PLAN
  I don't know of any automated test suites comparing UI components. I have 
already posted screenshots in the original bug report and can post additional 
ones of the new state for comparison. Apart from that, my manual sparse testing 
makes me feel the dark GTK theme looks and feels much more consistent to Breeze 
now.

REPOSITORY
  R98 Breeze for Gtk

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

AFFECTED FILES
  Breeze-dark-gtk/gtk-3.20/gtk.css
  Breeze-gtk/gtk-3.20/common.css
  Breeze-gtk/gtk-3.20/gtk.css

To: grmat
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14068: add -P switch to one-shot redshift call

[mat gr]

grmat added a comment.


  I was hoping the bug would get linked in automatically... anyways, here it 
is, with additional information: https://bugs.kde.org/show_bug.cgi?id=395641

REPOSITORY
  R885 Redshift Control for Plasma

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

To: grmat
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D14068: add -P switch to one-shot redshift call

[mat gr]

grmat created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
grmat requested review of this revision.

REVISION SUMMARY
  This trivial patch adds the missing parameter to get the familiar behaviour 
working for redshift versions since 1.12.
  
  BUG: 395641

REPOSITORY
  R885 Redshift Control for Plasma

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

AFFECTED FILES
  package/contents/ui/main.qml

To: grmat
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart