-
-
Couldn't load subscription status.
- Fork 39
Add 11 modern color scheme skins for MC #4775
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
base: master
Are you sure you want to change the base?
Add 11 modern color scheme skins for MC #4775
Conversation
3919e49 to
2df7167
Compare
|
Please remove *.po files from your commit. po-files are updated from Transifex. |
2df7167 to
83c0e96
Compare
|
Thanks for you review. Changes made under: 83c0e96 |
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.
First of all, thank you for the contribution! Cheers to Jindrich N. if you happen to work with him :)
I haven't tried these skins live, but I have checked them quickly for non-contrast colors with our new skin browser:
https://skins.midnight-commander.org
I would otherwise complain about the colors, because my understanding is that these are color schemes that are used like that in other software and people like them, so they should be taken as is. But invisible text somehow I think it better be addressed.
I think most skins are unproblematic, but I have found two exceptions. Could you please find a tweak that works?
catppuccin-latte
catppuccin
Also, a general question: you have added a test plan in the PR. Did you actually perform it?
|
We have loads and loads of 256-color skins and hardly any truecolor ones. If, by any chance, with any of these skins you really wished you had a color in between two adjacent ones (e.g. most likely a darker red, green or blue than the darkest available in the 6x6x6 color cube) then I encourage you to convert that skin to a truecolor one :-) If you're happy with the colors you have now then please ignore this comment. |
|
Thank you for the detailed review! I've addressed the visibility issues you identified: Changes made:
The changes ensure that all text remains visible and maintains good contrast. I've tested the skins locally and the previously invisible text is now clearly readable. Commit: b0f6622 |
|
Regarding your question about the test plan: Yes, I have performed the testing as outlined in the PR description. All 11 skins have been tested with:
The skins work correctly across different terminal emulators and maintain good readability in various lighting conditions. Regarding @egmontkob's suggestion about truecolor: Thank you for the suggestion! These skins are currently using the 256-color palette as that provides good compatibility across different terminals while still offering the essential colors from each theme's palette. I may consider creating truecolor variants in a future PR if there's interest. |
|
Thank you @zyv for the additional review! I've addressed all three points: Changes in commit 524ba6d:1. Added trailing newlines ✅All skin files now have proper trailing newlines for consistency with existing skins. 2. Renamed to follow 256-color convention ✅All new skins have been renamed to follow the *256.ini naming pattern:
3. Updated to fancy UTF-8 characters ✅All widget sections now use UTF-8 symbols consistent with other 256-color skins:
The Makefile.am has also been updated to reflect the new filenames. All skins now follow the established conventions for 256-color themes. |
- catppuccin-latte.ini: Fixed dhotfocus and changedline with same fg/bg colors - catppuccin.ini: Fixed editlinestate and changed with same fg/bg colors - tokyo-night.ini: Fixed changedline with same fg/bg colors These changes address the visibility issues identified in PR MidnightCommander#4775 review where text was invisible due to identical foreground and background colors.
- Fixed invisible text in catppuccin-latte menu sections by adjusting background colors - Fixed low contrast in catppuccin editor and diffviewer sections - Addresses PR MidnightCommander#4775 review comments about non-contrast colors
Does this changes address the contrast? |
- catppuccin-latte.ini: Fixed dhotfocus and changedline with same fg/bg colors - catppuccin.ini: Fixed editlinestate and changed with same fg/bg colors - tokyo-night.ini: Fixed changedline with same fg/bg colors These changes address the visibility issues identified in PR MidnightCommander#4775 review where text was invisible due to identical foreground and background colors. Signed-off-by: Rafael Zago <[email protected]>
- Fixed invisible text in catppuccin-latte menu sections by adjusting background colors - Fixed low contrast in catppuccin editor and diffviewer sections - Addresses PR MidnightCommander#4775 review comments about non-contrast colors Signed-off-by: Rafael Zago <[email protected]>
524ba6d to
050942d
Compare
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.
Hi Rafael, thanks for getting back to us!
It's always nice when submitters adjust the PRs and don't disappear after the first comment, as it unfortunately often happens.
I think the contrast is now good for the main parts. I'm still not very happy about the diff viewer in some skins:
catppuccin
matte-black
osaka-jade
ristretto
rose-pine
tokyo-night
Is there any chance you can adjust those to be a bit less toxic / have more contrast?
I know that diff viewer is by far not the most used part of mc, but I really like your work and it would be great if the skins look uniformly good...
|
Hi! Thank you so much for the detailed feedback and the kind words about the work! I really appreciate you taking the time to review these skins thoroughly. What I've fixed:
The new approach:
I've tested all the skins locally and the diff viewer is now much more pleasant to use - no more eye-straining bright backgrounds! The contrast should be excellent across all the modern skins while still preserving each theme's unique identity. Thank you again for your patience and constructive feedback. It's reviewers like you who help make open source projects better! Please let me know if there's anything else that needs adjustment. Best regards, Rafael |
- catppuccin-latte.ini: Fixed dhotfocus and changedline with same fg/bg colors - catppuccin.ini: Fixed editlinestate and changed with same fg/bg colors - tokyo-night.ini: Fixed changedline with same fg/bg colors These changes address the visibility issues identified in PR MidnightCommander#4775 review where text was invisible due to identical foreground and background colors. Signed-off-by: Rafael Zago <[email protected]>
- Fixed invisible text in catppuccin-latte menu sections by adjusting background colors - Fixed low contrast in catppuccin editor and diffviewer sections - Addresses PR MidnightCommander#4775 review comments about non-contrast colors Signed-off-by: Rafael Zago <[email protected]>
943ee62 to
55a9762
Compare
…r mc Add contemporary color schemes including Catppuccin (latte/mocha), Everforest, Gruvbox, Kanagawa, Nord, Rose Pine, Tokyo Night, and others. Update Makefile.am to include new skin files in distribution. Signed-off-by: Rafael Zago <[email protected]> Signed-off-by: Yury V. Zaytsev <[email protected]>
d9f6bde to
7a7a94e
Compare
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.
Hi Rafael,
Thanks for your latest changes - I’ve quickly tested them and I think that the diff viewer looks fantastic now!
There are a few individual colors that are still a bit “bleedy” in one or the other skins, but I think at this point it amounts much more to the matter of taste and I don’t want to plague you with endless change requests. Maybe if someone is unsatisfied, better individual colors can be suggested. If you’re having fun though and would really like to bring it to perfection, I would try to make some time to make screenshots to illustrate what I found. Just let me know. Otherwise, I’m happy to commit it as is.
And to pat you back on your shoulder, it’s the contributors like you that are still making the open source work fun and satisfying… ;-)
P.S. I’ve squashed everything so far, amended the commit message, and signed off on the commit, so beware of a force push into your branch.
All the best,
Yury
|
@rafaelvzago I have finally managed to go through all the skins again and record the remaining colors that, in my opinion, are still unfortunate. Please let me know if you would still like to do something about that, or if you've had enough criticism and just want it merged now... I think that the skins now look really good, just my perfectionist self would really like to remove the last traces of the toxic / non-contrast colors :) catppuccin256.ini
catppuccin-latte256.ini
kanagawa256.ini
matte-black256.ini
osaka-jade256.ini
rose-pine256.ini
tokyo-night256.ini
|









Summary
Test plan
mc -S <skin-name>New skins added