Allow changing the foreground/text color of buttons
ClosedPublic

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

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.Feb 8 2019, 4:09 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptFeb 8 2019, 4:09 PM
thiagoneves requested review of this revision.Feb 8 2019, 4:09 PM
cfeck added a comment.Feb 8 2019, 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.EditedFeb 8 2019, 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.Feb 8 2019, 5:15 PM
thiagoneves updated this revision to Diff 51249.Feb 9 2019, 1:24 PM

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

cfeck added inline comments.Feb 23 2019, 2:28 PM
kcalc.cpp
2076

I didn't want you to remove the complete code. If the colors are not changed, no style sheets should be set. See bug 237513.

kcalc_button.cpp
171

Better modify a local QColor variable before setting. This way you can remove the setEnabled() overload.

ping?

Hi, I was trying to tell that I removed that code because it was making the configuration not being applied asap when the user clicks the ok button.

kcalc.cpp
2076

Hi, I removed because It was affecting the config behaviour. You can reproduce by changing colors and saving. The application will only change when you restart it. This way, no need to restart anymore.

cfeck accepted this revision.Jul 10 2019, 9:18 PM

I will commit an updated version, restoring the check for bug 237513.

This revision is now accepted and ready to land.Jul 10 2019, 9:18 PM
This revision was automatically updated to reflect the committed changes.