Skip to content
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

feat: edit error/warning messages color of message logger #1247

Merged
merged 7 commits into from
Dec 16, 2023

Conversation

brun0-matheus
Copy link
Contributor

feat: Add option to choose error/warning message colors of message logger

Description

Add two new settings: "Error Message Color" and "Warn Message Color".
These settings are located at Appearance -> General, and have a combo box to select the colors, using default Qt Colors names.
Message Logger code was modified to use these settings instead of hardcoded values.

Related Issues / Pull Requests

Closes #521

Motivation and Context

Some users don't like the default colors for various reasons, so, an option to change them was added.

How Has This Been Tested?

Tested on Fedora Workstation 37, on dark and light modes.

Screenshots (if appropriate)

Checklist

  • If the key of a setting is changed, the old attribute is updated or it is resolved in SettingsUpdater.
  • If there are changes of the text displayed in the UI, they are wrapped in tr() or QCoreApplication::translate().
  • If needed, I have opened a pull request or an issue to update the documentation.
  • If these changes are notable, they are documented in CHANGELOG.md.

Additional text

It's necessary to translate the settings description and also add in the documentation and changelog.

Add two new settings: "Error Message Color" and "Warn Message Color".
These settings are located at Appeareance -> General, and have a
combo box to select the colors, using default Qt Colors names
(see https://doc.qt.io/qt-5/stylesheet-reference.html#color).

Addresses the issue cpeditor#521
Use the settings "Error Message Color" and "Warn Message Color"
to retrieve the error/warning colors instead of using hardcoded
ones.

Addresses issue cpeditor#521
@brun0-matheus brun0-matheus changed the title Edit messages color feat: Edit messages color Nov 2, 2023
@brun0-matheus brun0-matheus changed the title feat: Edit messages color feat: edit messages color Nov 2, 2023
@brun0-matheus brun0-matheus changed the title feat: edit messages color feat: edit error/warning messages color of message logger Nov 2, 2023
@coder3101
Copy link
Member

coder3101 commented Nov 3, 2023

Everything looks good!
However few suggestions before we merge this:

  • Update the translations file. There is a script for that, see the action that failed above.
  • Please check all of the combination of text colors and UI themes have enough contrast so people can read.
  • MacOS static checks failed, you can fix it, although it's not related to your PR, looks like static checker got updated/improved.
  • Update the changelog file.
  • Accompanying documentation changes PR would be nice as well.

Add to changelog the option to choose the error/warning message
color of message logger.
Update the translation files to address the texts of the two
new settings "Error Message Color" and "Warn Message Color"
@brun0-matheus
Copy link
Contributor Author

About the contrast of text color and background, I've tested all the combinations, and most of them looks good to me, except for ones like dark text on dark background or white text on white background. The combinations that don't look good to me are:

For white background: lime, aqua, silver, yellow, white text color.
For black background: teal, purple, navy, maroon, black text color

@brun0-matheus
Copy link
Contributor Author

The documentation pull request will be made soon. And about the mac os one, I will look into it in the near future (after docs).

Copy link

stale bot commented Dec 15, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Inactive issues label Dec 15, 2023
@ouuan ouuan added the work in progress The work has been started and is in progress. label Dec 15, 2023
@stale stale bot removed the stale Inactive issues label Dec 15, 2023
@coder3101
Copy link
Member

coder3101 commented Dec 15, 2023

#1268 should unblock this PR, @brun0-matheus please rebase the changes on top of master.

EDIT: Done

@coder3101 coder3101 enabled auto-merge (squash) December 16, 2023 14:22
@coder3101 coder3101 requested a review from ouuan December 16, 2023 14:22
CHANGELOG.md Outdated Show resolved Hide resolved
@coder3101 coder3101 merged commit 42f0532 into cpeditor:master Dec 16, 2023
8 checks passed
@coder3101
Copy link
Member

@all-contributors please add @brun0-matheus for code

@coder3101 coder3101 removed the work in progress The work has been started and is in progress. label Dec 16, 2023
Copy link
Contributor

@coder3101

I've put up a pull request to add @brun0-matheus! 🎉

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

Successfully merging this pull request may close these issues.

Option to change the color of messages shown in the message logger
3 participants