-
Notifications
You must be signed in to change notification settings - Fork 188
Improvement: Updated the Default Colors for Negative, Positive, and Warning Events to Display Better in Dark Themes #6842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…arning Events to Display Better in Dark Themes
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6842 +/- ##
============================================
- Coverage 11.46% 11.45% -0.02%
+ Complexity 6508 6505 -3
============================================
Files 1098 1099 +1
Lines 140812 140898 +86
Branches 21831 21838 +7
============================================
- Hits 16150 16139 -11
- Misses 122995 123095 +100
+ Partials 1667 1664 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the default color codes to improve the visual display of Negative, Positive, and Warning events in dark themes.
- Updated default negative, positive, and warning color values in MHQOptions.java
- Added license information rega 8000 rding Microsoft's Game Content Usage Rules
@@ -572,7 +577,7 @@ public void setStratConHexCoordForeground(Color value) { | |||
|
|||
public Color getFontColorNegative() { | |||
return new Color(userPreferences.node(MHQConstants.DISPLAY_NODE) | |||
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, Color.RED.getRGB())); | |||
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, 0xDF5341)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider defining a named constant for the default negative font color to improve code maintainability and clarity.
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, 0xDF5341)); | |
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, DEFAULT_FONT_COLOR_NEGATIVE)); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While that would be reasonable, I don't want to go through and refactor that much as it wouldn't make sense to just do it for a couple of colors, we'd want to roll it out across all colors.
@@ -588,7 +593,7 @@ public void setFontColorNegative(Color value) { | |||
|
|||
public Color getFontColorPositive() { | |||
return new Color(userPreferences.node(MHQConstants.DISPLAY_NODE) | |||
.getInt(MHQConstants.FONT_COLOR_POSITIVE, Color.GREEN.getRGB())); | |||
.getInt(MHQConstants.FONT_COLOR_POSITIVE, 0x7FCF43)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider defining a named constant for the default positive font color to enhance maintainability and ease potential future adjustments.
.getInt(MHQConstants.FONT_COLOR_POSITIVE, 0x7FCF43)); | |
.getInt(MHQConstants.FONT_COLOR_POSITIVE, DEFAULT_FONT_COLOR_POSITIVE)); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to de 8000 scribe this comment to others. Learn more.
While that would be reasonable, I don't want to go through and refactor that much as it wouldn't make sense to just do it for a couple of colors, we'd want to roll it out across all colors.
@@ -604,7 +609,7 @@ public void setFontColorPositive(Color value) { | |||
|
|||
public Color getFontColorWarning() { | |||
return new Color(userPreferences.node(MHQConstants.DISPLAY_NODE) | |||
.getInt(MHQConstants.FONT_COLOR_WARNING, Color.ORANGE.getRGB())); | |||
.getInt(MHQConstants.FONT_COLOR_WARNING, 0xEE9A00)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider abstracting the default warning font color into a constant to improve consistency and simplify updates in the future.
.getInt(MHQConstants.FONT_COLOR_WARNING, 0xEE9A00)); | |
.getInt(MHQConstants.FONT_COLOR_WARNING, DEFAULT_WARNING_FONT_COLOR)); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While that would be reasonable, I don't want to go through and refactor that much as it wouldn't make sense to just do it for a couple of colors, we'd want to roll it out across all colors.
Reviewed by me and aprooved |
This PR updates the default colors for Negative (red), Positive (green), and Warning (orange) events to be less garish in dark themes.
Note
It seems that we're storing these colors somewhere weird that I've not been able to track down, so if you're launching from your IDE you're probably stuck with the old colors. If you find out where these values are stored, please let me know.