Allow changing the foreground/text color of buttons
Needs ReviewPublic

Authored by thiagoneves on Fri, Feb 8, 4:09 PM.

Details

Reviewers
cfeck

Diff Detail

Repository
R353 KCalc
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8063
Build 8081: arc lint + arc unit
thiagoneves created this revision.Fri, Feb 8, 4:09 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptFri, Feb 8, 4:09 PM
thiagoneves requested review of this revision.Fri, Feb 8, 4:09 PM
cfeck added a comment.Fri, Feb 8, 5:03 PM

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.

kcalc.cpp
2076

No need to create a color scheme twice. Please reuse schemeButtons for button and text color.

2082

Please change casts to qobject_cast (here and below).

kcalc_button.cpp
168

This change can be reverted. The palette.setColor() call below is sufficient.

171

textColor() returns a QColor, so the cast is not needed.

232

Color does not affect the size. Can be removed.

kcalc_button.h
70

QColor textColor() const

93

Please rename to text_color_ to make it consistent with other member variables.

cfeck requested changes to this revision.EditedFri, Feb 8, 5:15 PM

There is a usability issue with the MR button when the memory is empty. Setting the text color overrides the lighter color that is used for disabled buttons.

I suggest to use something like setAlphaF(0.7) on the color for disabled() buttons before modifying the context's palette.

This revision now requires changes to proceed.Fri, Feb 8, 5:15 PM
thiagoneves updated this revision to Diff 51249.Sat, Feb 9, 1:24 PM

Updating D18854: Allow changing the foreground/text color of buttons