D16353: Change color(NegativeBackground) of status bar in space info when storage exceeds 90%

[Méven Car]

meven added a comment.


  In D16353#613832 <https://phabricator.kde.org/D16353#613832>, @cfeck wrote:
  
  > Sorry, it is still wrong.
  
  
  I am glad to learn.
  
  > QPalette and KColorScheme have a fixed set of compatible "bg + fg" colors.
  > 
  > For QPalette:
  > 
  > - Base/AlternateBase + Text
  > - Window + WindowText
  > - Button + ButtonText
  > - Highlight + HighlightedText
  > - ToolTipBase + ToolTipText (note that Background + Foreground is an obsolete way to express Window + WindowText)
  > 
  >   For KColorScheme, there are many more compatible combinations. Basically, for each "set" of colors (View, Window, Button, Selection, ToolTip, which map to the QPalette backgrounds above), there are many separate background roles and matching foreground/text roles.
  
  Thanks I did not know those (implicit?) rules (do we have documentation ?).
  
  > So the statement `palette.setBrush(QPalette::Highlight, colorScheme.foreground(KColorScheme::ActiveText))` is wrong, because you are trying to set a fg/text color as a background color. Same for `palette.setBrush(QPalette::Background, colorScheme.foreground(KColorScheme::NegativeText))`.
  
  
  
  In D16353#613832 <https://phabricator.kde.org/D16353#613832>, @cfeck wrote:
  
  > Sorry, it is still wrong. QPalette and KColorScheme have a fixed set of compatible "bg + fg" colors.
  >
  > For QPalette:
  >
  > - Base/AlternateBase + Text
  > - Window + WindowText
  > - Button + ButtonText
  > - Highlight + HighlightedText
  > - ToolTipBase + ToolTipText (note that Background + Foreground is an obsolete way to express Window + WindowText)
  >
  >   For KColorScheme, there are many more compatible combinations. Basically, for each "set" of colors (View, Window, Button, Selection, ToolTip, which map to the QPalette backgrounds above), there are many separate background roles and matching foreground/text roles.
  >
  >   So the statement `palette.setBrush(QPalette::Highlight, colorScheme.foreground(KColorScheme::ActiveText))` is wrong, because you are trying to set a fg/text color as a background color. Same for `palette.setBrush(QPalette::Background, colorScheme.foreground(KColorScheme::NegativeText))`.
  
  
  What would be the correct statement then ?
  
  If I understood you correctly I updated the code accordingly.
  
  But the colors are off, for instance with breeze.
  
  Negative Background:
  F8124682: Screenshot_20200222_112059.png <https://phabricator.kde.org/F8124682>
  
  Positive Background:
  F8124683: Screenshot_20200222_112250.png <https://phabricator.kde.org/F8124683>
  
  This was the reason I chose those values in the first place : I adapted the colors to those the :CapacityBar uses rather than semantics because this components don't use strictly the semantics.
  role, not a foreground color role :
  
  In D16353#595500 <https://phabricator.kde.org/D16353#595500>, @meven wrote:
  
  > Well as I said breeze uses `QPalette::Highlight` to draw CapacityBar ( rendered as ProgressBar by breeze) in breezestyle.cpp  `Style::drawProgressBarContentsControl( const QStyleOption* option, QPainter* painter, const QWidget* ) const`
  
  
  I had to find those based on KCapacityBar (palette().base()) and Breeze (Style::drawProgressBarContentsControl palette.color( QPalette::Highlight )) the QPalette colors they use.
  
  I don't care so much about semantics (as KCapacityBar and Breeze do) than if it works and would work for all themes.

REPOSITORY
  R318 Dolphin

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

To: meven, ngraham, pino, elvisangelaccio, #dolphin, sourabhboss
Cc: meven, cfeck, kfm-devel, pberestov, iasensio, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20200222/d0ef2b4a/attachment.htm>