8000 Improvement: Updated the Default Colors for Negative, Positive, and Warning Events to Display Better in Dark Themes by IllianiBird · Pull Request #6842 · MegaMek/mekhq · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 1 commit into from
May 2, 2025

Conversation

IllianiBird
Copy link
Collaborator
@IllianiBird IllianiBird commented May 2, 2025

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.

…arning Events to Display Better in Dark Themes
@IllianiBird IllianiBird added GUI UX User experience labels May 2, 2025
@IllianiBird IllianiBird self-assigned this May 2, 2025
Copy link
codecov bot commented May 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 11.45%. Comparing base (5edad64) to head (4562e50).
Report is 77 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@HammerGS HammerGS requested a review from Copilot May 2, 2025 03:28
Copy link
Contributor
@Copilot Copilot AI left a 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));
Copy link
Preview
Copilot AI May 2, 2025

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.

Suggested change
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, 0xDF5341));
.getInt(MHQConstants.FONT_COLOR_NEGATIVE, DEFAULT_FONT_COLOR_NEGATIVE));

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

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));
Copy link
Preview
Copilot AI May 2, 2025

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.

Suggested change
.getInt(MHQConstants.FONT_COLOR_POSITIVE, 0x7FCF43));
.getInt(MHQConstants.FONT_COLOR_POSITIVE, DEFAULT_FONT_COLOR_POSITIVE));

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

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));
Copy link
Preview
Copilot AI May 2, 2025

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.

Suggested change
.getInt(MHQConstants.FONT_COLOR_WARNING, 0xEE9A00));
.getInt(MHQConstants.FONT_COLOR_WARNING, DEFAULT_WARNING_FONT_COLOR));

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

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.

@IllianiBird IllianiBird marked this pull request as ready for review May 2, 2025 03:39
@Scoppio Scoppio merged commit fbf1d4a into MegaMek:master May 2, 2025
6 checks passed
@Scoppio
Copy link
Collaborator
Scoppio commented May 2, 2025

Reviewed by me and aprooved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI UX User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0