D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Thanks !

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Noah Davis]

ndavis abandoned this revision.
ndavis added a comment.


  Alright, problem fixed.

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Noah Davis]

ndavis added a comment.


  In D23169#513046 , 
@hpereiradacosta wrote:
  
  > Hi Noah
  >  Thanks for the patch, however, it is not the right fix to the issue. If 
you use a light color scheme (like the default breeze), you will see that the 
shadow below the part of the button that corresponds to the arrow is darker 
than below the rest of the button. This is because the frame is actually 
rendered twice.
  >
  > Now, the bug you try to fix is real, and as I was 100% sure that it was not 
there in the past, I used git bisect to track it down to this commit:
  >
  > 32d8b02880a237e6de415861500a018a5cd09781 

  >
  > The corresponding diff contains 
  >  @@ -5988,7 +5988,6 @@ namespace Breeze
  >
  >   // frame
  >   if( toolButtonOption->subControls & SC_ToolButton )
  >   {
  >
  > - copy.rect = buttonRect; if( inTabBar ) 
drawTabBarPanelButtonToolPrimitive( , painter, widget ); else 
drawPrimitive( PE_PanelButtonTool, , painter, widget); }
  >
  >   Which is what causes the issue. Could you revert this commit, and push 
instead the proper fix that I will post in another comment ?
  >
  >   Alternatively, I can do it myself. Note that your changes on the 
separator position is legit, but should be a different patch
  
  
  Sure, thanks for the help.

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Noah Davis]

ndavis added a comment.


  In D23169#513071 , 
@hpereiradacosta wrote:
  
  > Zoom showing the issue mentionned above with the "current" patch (or master 
branch of breeze)
  >
  > F7250494: Screenshot_20190816_175237.png 

  >
  > How it should look: 
  >  F7250505: Screenshot_20190816_175628.png 

  
  
  Whoops. Is that a double shadow?

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Zoom showing the issue mentionned above with the "current" patch (or master 
branch of breeze)
  
  F7250494: Screenshot_20190816_175237.png 

  
  How it should look: 
  F7250505: Screenshot_20190816_175628.png 


REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Hugo Pereira Da Costa]

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


  F7250479: patch.diff 

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi Noah
  Thanks for the patch, however, it is not the right fix to the issue. If you 
use a light color scheme (like the default breeze), you will see that the 
shadow below the part of the button that corresponds to the arrow is darker 
than below the rest of the button. This is because the frame is actually 
rendered twice.
  
  Now, the bug you try to fix is real, and as I was 100% sure that it was not 
there in the past, I used git bisect to track it down to this commit:
  
  32d8b02880a237e6de415861500a018a5cd09781 

  
  The corresponding diff contains 
  @@ -5988,7 +5988,6 @@ namespace Breeze
  
// frame
if( toolButtonOption->subControls & SC_ToolButton )
{
  
  - copy.rect = buttonRect; if( inTabBar ) drawTabBarPanelButtonToolPrimitive( 
, painter, widget ); else drawPrimitive( PE_PanelButtonTool, , 
painter, widget); }
  
  Which is what causes the issue. 
  Could you revert this commit, and push instead the proper fix that I will 
post in another comment ? 
  Thanks !

REPOSITORY
  R31 Breeze

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

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

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Noah Davis]

This revision was automatically updated to reflect the committed changes.
Closed by commit R31:b43e19e3e13c: Fix width and separator of 
ToolButtonComplexControl outline w/ dropdown menu (authored by ndavis).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23169?vs=63783=63840

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: ndavis, #vdg, #breeze, ngraham
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D23169: Fix width and separator of ToolButtonComplexControl outline w/ dropdown menu

[Nathaniel Graham]

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


  Thanks for fixing this annoying issue.

REPOSITORY
  R31 Breeze

BRANCH
  fix-ToolButtonComplexControl (branched from master)

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

To: ndavis, #vdg, #breeze, ngraham
Cc: ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart