Differential D12392

Fix the "Default" color scheme to match Breeze again
ClosedPublic
Actions

Authored by ngraham on Apr 21 2018, 4:13 AM.

Details

Reviewers
hein
Group Reviewers
Breeze
Plasma
Commits
R265:ebdb8424cd12: Fix the "Default" color scheme to match Breeze again
Summary

Over the years, the "Default" color scheem seems to have drifted out of alignment with Breeze. Part of this was my doing, in https://cgit.kde.org/breeze.git/commit/?id=04dac01c2a3268f7f6189b2d5c404cb2b7aaf51f, but there were other changes too that were not reflected in the Default color scheme.

This patch fixes it to match Breeze again. There may be more patches required in other repos, which will be forthcoming as I discover more definitions of the default color scheme (this is the only once I've found so far).

Test Plan

Diff Detail

Repository
R265 KConfigWidgets
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Apr 21 2018, 4:13 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 21 2018, 4:13 AM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
ngraham requested review of this revision.Apr 21 2018, 4:13 AM
ngraham edited the test plan for this revision. (Show Details)Apr 21 2018, 4:13 AM
ngraham edited the summary of this revision. (Show Details)
ngraham added a comment.May 2 2018, 5:45 PM

Friendly ping!

abetts added a subscriber: abetts.May 2 2018, 5:49 PM

Just trying to understand. What is happening here is that we have 3 color schemes that are all theoretically the same as Breeze. However, the "default" and "Breeze" scheme are currently not the same. Therefore, we have to make sure that they are. If that's the case

+1

ngraham added a comment.May 2 2018, 6:24 PM

"Current" is whatever the current theme is and can be ignored (and hopefully gotten rid of in the future). The issue here is that "Default" is supposed to be identical to "Breeze", but it's currently not. This patch fixes that.

cfeck added a subscriber: cfeck.May 2 2018, 6:27 PM

"Current" could be named "Unnamed" or "Unsaved". It cannot be removed, unless you want to force the user to always create a named color scheme when he does modifications.

ngraham added a comment.May 2 2018, 6:41 PM

The user already has to create a named color scheme if he wants to actually save and use his modifications.

ngraham added a comment.EditedMay 10 2018, 6:55 PM

Ping? Shall I take silence to mean tacit acceptance?

Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptMay 10 2018, 6:55 PM
hein accepted this revision.May 13 2018, 7:34 AM

This looks good, but I think there might be additional copies of these values in (a) the color KCM code and (b) the kdeglobals code in kdelibs4support, did you have a chance to look into those?

This revision is now accepted and ready to land.May 13 2018, 7:34 AM
ngraham added a comment.May 13 2018, 11:32 PM

Thanks! I didn't find anything in kdelibs or the color KCM code, actually. The Colors KCM has several other color schemes listed in there, but not a copy of Breeze.

Closed by commit R265:ebdb8424cd12: Fix the "Default" color scheme to match Breeze again (authored by ngraham). · Explain WhyMay 13 2018, 11:37 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/kcolorscheme.cpp (24 lines)

Diff 34085

View Options

src/kcolorscheme.cpp

Loading...