Allow changing the foreground/text color of buttons
ClosedPublic

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

Diff Detail

Repository
R353 KCalc
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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
2146

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

2153

Please change casts to qobject_cast (here and below).

kcalc_button.cpp
160

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

163–167

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

228

Color does not affect the size. Can be removed.

kcalc_button.h
70

QColor textColor() const

91

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
2146

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
163–167

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
2146

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.Wed, Jul 10, 9:18 PM

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

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