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