D18854: Allow changing the foreground/text color of buttons
    Christoph Feck 
    noreply at phabricator.kde.org
       
    Fri Feb  8 17:03:26 GMT 2019
    
    
  
cfeck added a comment.
  Could you change the UI to have three columns? It already uses a grid layout, so you can insert a third column.
  
    Button colors
    Functions: [FG] [BG]
    Numbers:  [FG] {BG]
  
  etc.
  
  This way it is easier to see the color combinations.
INLINE COMMENTS
> kcalc.cpp:2080
> +
> +	KColorScheme schemeFonts(QPalette::Active, KColorScheme::Button);
> +    const QColor defaultFontColor = schemeFonts.foreground().color();
No need to create a color scheme twice. Please reuse `schemeButtons` for button and text color.
> kcalc.cpp:2103
> +        (num_button_group_->button(i))->setStyleSheet(sheet.arg(numPal.name()));
> +        dynamic_cast<KCalcButton*>((num_button_group_->button(i)))->setTextColor(numFontColor);
>  	}
Please change casts to `qobject_cast` (here and below).
> kcalc_button.cpp:164
>  	QAbstractTextDocumentLayout::PaintContext context;
> -	doc.setHtml(QLatin1String("<center>") + text() + QLatin1String("</center>"));
> +	doc.setHtml(QLatin1String("<center><font color=") + textColor().name() + QLatin1String(">")  + text() + QLatin1String("</font></center>"));
>  	doc.setDefaultFont(font());
This change can be reverted. The `palette.setColor()` call below is sufficient.
> kcalc_button.cpp:167
>  	context.palette = palette();
> -	context.palette.setColor(QPalette::Text, context.palette.buttonText().color());
> +	context.palette.setColor(QPalette::Text, QColor(textColor()));
>  
textColor() returns a QColor, so the cast is not needed.
> kcalc_button.cpp:228
> +	mTextColor = color;
> +	calcSizeHint();
> +}
Color does not affect the size. Can be removed.
> kcalc_button.h:70
>      QSize sizeHint() const override;
> +    QColor textColor();
> +    void setTextColor(const QColor &color);
`QColor textColor() const`
> kcalc_button.h:92
>      QSize                             size_;
> +    QColor                            mTextColor;
>  };
Please rename to `text_color_` to make it consistent with other member variables.
REPOSITORY
  R353 KCalc
REVISION DETAIL
  https://phabricator.kde.org/D18854
To: thiagoneves, cfeck
Cc: kde-utils-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20190208/22cd955f/attachment-0001.html>
    
    
More information about the Kde-utils-devel
mailing list