D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Hugo Pereira Da Costa 
This revision was automatically updated to reflect the committed changes.
Closed by commit R31:d6b2a3a36a1c: Remove unneeded 1 pixel margin around side 
panels (authored by hpereiradacosta).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22138?vs=61228=61236

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

AFFECTED FILES
  kstyle/breezehelper.cpp
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Noah Davis 
ndavis added a comment.


  Works for me. Tested RTL with `kate --reverse`.

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Hugo Pereira Da Costa 
hpereiradacosta updated this revision to Diff 61228.
hpereiradacosta added a comment.


  Simplified the patch: special case in ::pixelMetrics is in fact not needed, 
because it is overridden in ::frameContentsRect anyway

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22138?vs=61227=61228

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

AFFECTED FILES
  kstyle/breezehelper.cpp
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Hugo Pereira Da Costa 
hpereiradacosta updated this revision to Diff 61227.
hpereiradacosta added a comment.


  New patch, adding proper margin to avoid overlap with viewport

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22138?vs=60773=61227

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

AFFECTED FILES
  kstyle/breezehelper.cpp
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  In D22138#491285 , @mart wrote:
  
  > In D22138#491173 , 
@hpereiradacosta wrote:
  >
  > > Indeed. This is a problem. As soon as treeviews are animated Qt makes a 
pixmap copy of the treeview viewport. This does not include the blue line on 
the side, which is painted in the frame, below the (transparent) viewport. Will 
investigate further.
  >
  >
  > would still adding a pixel on the right work around the issue?
  
  
  Yes, left or right depending of RTL. I have a working patch (I think). Will 
upload in a minute.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Marco Martin 
mart added a comment.


  In D22138#491173 , 
@hpereiradacosta wrote:
  
  > Indeed. This is a problem. As soon as treeviews are animated Qt makes a 
pixmap copy of the treeview viewport. This does not include the blue line on 
the side, which is painted in the frame, below the (transparent) viewport. Will 
investigate further.
  
  
  would still adding a pixel on the right work around the issue?

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-05 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  Indeed. This is a problem. As soon as treeviews are animated Qt makes a 
pixmap copy of the treeview viewport. This does not include the blue line on 
the side, which is painted in the frame, below the (transparent) viewport. Will 
investigate further.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Noah Davis 
ndavis requested changes to this revision.
ndavis added a comment.
This revision now requires changes to proceed.


  Actually, I did find one problem that I missed from before: F6971880: 
sidebar_tree-2019-07-05_00.04.24.webm 

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Noah Davis 
ndavis accepted this revision.
ndavis added a comment.
This revision is now accepted and ready to land.


  As far as I can tell, there aren't any problems with this patch. Some apps 
like Spectacle and the system tray settings don't follow the new style, but 
they don't use Qt Widgets for the sidebar.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Noah Davis 
ndavis added a comment.


  In D22138#491018 , 
@hpereiradacosta wrote:
  
  > In D22138#490967 , @ndavis wrote:
  >
  > > Since this apparently doesn't need review,
  >
  >
  > This is not what I said. What I said is "thanks, it looks fantastic" is not 
a proper review and hence irrelevant. What I expect for a proper review is
  >
  > - does the patch achieve the behavior matching its title, and is it the 
intended behavior
  > - does the code look legit (compiles, properly formatted, etc.)
  > - is there a better way to achieve the same
  > - does the patch break other things, and how can it be improved so that it 
doesn't If you can provide such a review I'd be happy to modify the patch 
accordingly.
  >
  > > I'll resign.
  
  
  In that case, I will review this.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  In D22138#490976 , @ngraham wrote:
  
  > Very nice! In conjunction with Marco's patch (D22083 
), I now see the following for Dolphin's 
settings window: F6968869: Screenshot_20190704_134502.png 

  >
  > I notice that your screenshot depicts the sidebar with no top, bottom, or 
left margins, which is the indended appearance. Is that the result of some 
other required patch I haven't applied yet, or did I do something wrong?
  
  
  Sorry. My bad. Yes, my own copy included other changes beyond my patch and 
Marco's, resulting in the zero margins. The screenshot you post is the correct 
one. Thanks. 
  The remaining margin issue, handled at the layout level, either in breeze or 
in kpagedialog, should be a separate patch.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  In D22138#490967 , @ndavis wrote:
  
  > Since this apparently doesn't need review,
  
  
  This is not what I said. What I said is "thanks, it looks fantastic" is not a 
proper review and hence irrelevant. What I expect for a proper review is
  
  - does the patch achieve the behavior matching its title, and is it the 
intended behavior
  - does the code look legit (compiles, properly formatted, etc.)
  - is there a better way to achieve the same
  - does the patch break other things, and how can it be improved so that it 
doesn't
  
  If you can provide such a review I'd be happy to modify the patch accordingly.
  
  > I'll resign.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Nathaniel Graham 
ngraham added a comment.


  Very nice! In conjunction with Marco's patch (D22083 
), I now see the following for Dolphin's 
settings window: F6968869: Screenshot_20190704_134502.png 

  
  I notice that your screenshot depicts the sidebar with no top, bottom, or 
left margins, which is the indended appearance. Is that the result of some 
other required patch I haven't applied yet, or did I do something wrong?

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-07-04 Thread Noah Davis 
ndavis resigned from this revision.
ndavis added a comment.


  Since this apparently doesn't need review, I'll resign.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-06-28 Thread Nathaniel Graham 
ngraham added a comment.


  Umm OK, sorry if I've said something wrong.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-06-28 Thread Hugo Pereira Da Costa 
hpereiradacosta added a comment.


  In D22138#487668 , @ngraham wrote:
  
  > Thanks @hpereiradacosta! This looks fantastic. Adding @ndavis and @filipf 
for comment since they've been working on this project too from other angles.
  
  
  No need for thanks @ngraham, I am not doing this for you and this is 
irrelevant in terms of review.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-06-28 Thread Nathaniel Graham 
ngraham added subscribers: filipf, ndavis, ngraham.
ngraham added reviewers: VDG, ndavis, filipf.
ngraham added a comment.


  Thanks @hpereiradacosta! This looks fantastic. Adding @ndavis and @filipf for 
comment since they've been working on this project too from other angles.

REPOSITORY
  R31 Breeze

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

To: hpereiradacosta, mart, #vdg, ndavis, filipf
Cc: ngraham, ndavis, filipf, mart, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D22138: Remove 1 pixel margin around side panels, use QPalette::Base for background

2019-06-28 Thread Hugo Pereira Da Costa 
hpereiradacosta created this revision.
hpereiradacosta added a reviewer: mart.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
hpereiradacosta requested review of this revision.

REVISION SUMMARY
  This patch removes unneeded 1 pixel margin around side panels (namely 
QAbstractScrollArea with property _kde_side_panel_view set to true), and make 
the background QPalette::Base
  The viewport background is kept transparent, in order to allow for a vertical 
line to be drawn below it (in the frame), without being hidden behind the it. 
  Screenshot: F6939900: Screenshot_20190628_142810.png 


REPOSITORY
  R31 Breeze

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

AFFECTED FILES
  kstyle/breezehelper.cpp
  kstyle/breezestyle.cpp

To: hpereiradacosta, mart
Cc: mart, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol