Colour definitions as enumeration classes #4405
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to improve the type safety using enumeration classes rather #defines within the GC Color definitions, this PR makes no change to GC's Colors or appearance. The following describes the changes and a few defects detected & corrected by this PR:
Note: The main changes are within Colors.[h& cpp] and Pages.cpp, all the other file changes are related #defines changing to enumerated class definitions.
⦁ GCColor has been made a singleton class, this enables the duplicate code within ColorsPage::applyThemeIndex(int index) & GCColor::applyTheme(int index) functions to be extracted a placed in a single function detemineThemeQColor() within GCColor, and this detected the following issue:
The two "duplicate" functionalities handled the Hover case differently, this PR implements the code below as it more closely matches the description, but I'm not sure which is the right answer:
case GCol::HOVER:
// stealthy themes use overview card background for hover color since they are close
// all other themes get a boring default
//qColor = theme.stealth ? colorTable[gColor].qColor : (theme.dark ? QColor(50, 50, 50) : QColor(200, 200, 200));
qColor = theme.stealth ? theme.colors[GThmCol::OVERVIEWTILEBACKGROUND1] : (theme.dark ? QColor(50, 50, 50) : QColor(200, 200, 200));
break;
`
⦁ Replaces the #defines for Colors with an enumeration class (GCol) to improve type safety, this change has led to numerous trivial changes in many files, but also detected the following issues fixed by this PR:
OverviewItems.h
A gc color should be passed to the GColor macro not a qt color, so this is now:
as Qt::gray has the value of 5, so its replaced with RIDEPLOTXAXIS which has the same value.
ExtendedCriticalPower.cpp
A gc color should be passed to the GColor macro not a qt color, so this is now:
as Qt::lightGray has the value of 6, so its replaced with RIDEPLOTYAXIS which has the same value.
⦁ Replaces the theme number indexes with an enumeration class (GThmCol) to improve type safety and makes it easy to determine whether the wrong index is being used.
becomes:
⦁ Error found in GColor function readConfig, the name field is a text desciption of the gc color, but it is being compared against the setting value, therefore these defaults will never be applied:
This has been corrected to:
⦁ The three Lists: ColorList, LightDefaultColorList & DarkDefaultColorList have been combined into a single colorTable definition within GCColor using a std::map<GCol, Colors>, where all fields in the Colors class except the qColor entry are constants. This removes the previous list duplication, and simplifies checking the differences between light & dark for each defined gc color.
⦁ The RGB conversion macros have been converted to functions to enforce type safety:
⦁ The "standard" Colors have been moved within GCColor.
⦁ Functions within GCColor class have been logically grouped in the header file.
⦁ Variable names within GCColors.[h&cpp] have been consistently named either gColor or qColor to clarify their type.
⦁ The unused "struct SizeSettings" in GColor.h has been removed.
⦁ The GCColor::isFlat() function only ever returned true, so has been removed
⦁ All the swatches within a theme are now displayed in the Colors Page window rather the first five, allowing the differences between similar themes to be visualised.