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

Fix title bar text color and readme #1194

Closed
wants to merge 2 commits into from
Closed

Fix title bar text color and readme #1194

wants to merge 2 commits into from

Conversation

zpreston123
Copy link

@zpreston123 zpreston123 commented Oct 3, 2017

Description

I changed the default title bar text color the same as the folder label color in all theme variants. Also removed an extra space in the README settings that wasn’t done in my previous commit.

Motivation and Context

This would improve the title bar text for greater clarity. Hope this helps resolve issue #1190.

How Has This Been Tested?

This was tested using PackageDev, NPM, and Gulp to compile and view the modified theme. The README file was tested using dillinger.io markdown editor.

Screenshots (if appropriate):

Default (before):
default-before-titlebar
Default (after):
default-titlebar

Darker (before):
darker-before-titlebar
Darker (after):
darker-titlebar

Lighter (before):
lighter-before-titlebar
Lighter (after):
lighter-titlebar

Palenight (before):
palenight-before-titlebar
Palenight (after):
palenight-titlebar

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.

@equinusocio
Copy link
Member

Hi @zpreston123 The issue #1190 could be caused by High Sierra. Check this comment
#1190 (comment)

Before changing the UI we need to be sure that the theme is the problem and not any other things

@zpreston123
Copy link
Author

zpreston123 commented Oct 9, 2017

OK I read the comment, so it's very likely High Sierra is causing the issue. However, the Adaptive theme title bar seemed to render fine after I switched to one of the dark color schemes:

screen shot 2017-10-09 at 7 30 20 pm

If the issue doesn't get resolved in the next OS update or two, then I think this fix would suffice.

@mallowigi
Copy link

It’s because they use a white title. Some colors like red, cyan, yellow... are rendered just fine, but our shade of gray/purple doesn’t enter in this category.

A possible fix would be to change back to white, but it wouldn’t fit with the colors, and besides, it doesn’t happen to everyone

@zpreston123
Copy link
Author

zpreston123 commented Oct 22, 2017

@mallowigi I agree, white for the dark themes and black for the light theme wouldn't fit very well. I think the title bar along with these changes would be better as an option.

@equinusocio I can add an option to enable the title bar, which many users seem to want (#1202).

@equinusocio
Copy link
Member

Yes. Make it activable behind setting but leave the foreground color untouched. Thank you so much

@zpreston123
Copy link
Author

zpreston123 commented Oct 22, 2017

@equinusocio OK that works! You mean the default native foreground color or the same ones in these changes?

@equinusocio
Copy link
Member

The original material theme foreground (before your changes)

@zpreston123
Copy link
Author

@equinusocio Got it thanks!

@zpreston123 zpreston123 deleted the fix/titlebar-and-readme branch November 7, 2017 01:56
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.

3 participants