8000 (Theme): (Solarized Dark uses wrong text color) · Issue #5130 · ajaxorg/ace · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

(Theme): (Solarized Dark uses wrong text color) #5130

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.

Sign up for GitHub

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

Jump to bottom
Closed
doom-goober opened this issue Apr 15, 2023 · 5 comments
Closed

(Theme): (Solarized Dark uses wrong text color) #5130

doom-goober opened this issue Apr 15, 2023 · 5 comments
Labels
bug effort/small good first issue p2 theme-issue

Comments

@doom-goober
Copy link

Describe the bug

If you go to the original solarized web page you can find this blurb:

Thus in the case of a dark background colorscheme, the normal relationship for background and body text is base03:base0 (please note that body text is not base00).

base03 #002b36
base0 #839496
base00 #657b83
base1 #93a1a1

But solarized_dark.css.js defines the following as the primary text color:

.ace-solarized-dark {
  background-color: #002B36;
  color: #93A1A1
}

.ace-solarized-dark .ace_entity.ace_other.ace_attribute-name,
.ace-solarized-dark .ace_storage {
  color: #93A1A1
}

Notice Ace's version uses color, #93A1A1 while the Solarized web page explicitly states the text color should be: #839496

Expected Behavior

For Solarized Dark, the text color should be: #839496

Current Behavior

For Solarized Dark, the text color is: #93A1A1.

Reproduction Steps

Create an Ace Editor using Solarized Dark theming.

Possible Solution

Change the color value from #93A1A1 to #839496

Additional Information/Context

No response

Ace Version / Browser / OS / Keyboard layout

v1.17.0
gurvirbaraich added a commit to gurvirbaraich/ace that referenced this issue Apr 18, 2023
@gurvirbaraich
fixes ajaxorg#5130: Wrong text color in solarized_dark.css.js
71a6ebb
@gurvirbaraich gurvirbaraich mentioned this issue Apr 18, 2023
Closed
@gurvirbaraich
Copy link

Fixed the issue and also made a pull request

@gurvirbaraich
Copy link

The pull request has passed all the requests but I am not getting why its not merged. Its my first pull request and I don't know much about it.

@doom-goober
Copy link
Author
doom-goober commented Apr 21, 2023 •
edited
Loading

@gurvirbaraich I apologize, I made a small mistake. The following CSS block should probably retain its original color, base1:

.ace-solarized-dark .ace_entity.ace_other.ace_attribute-name,
.ace-solarized-dark .ace_storage {
color: #93A1A1
}

The Solarized Web Site describes base1 in Dark Mode as: "optional emphasized content". For JavaScript, that slightly emphasized the words like function in the code block "function hello() {}".

However, .ace-solarized-dark should definitely be: 839496.

Sorry about that. I dunno if you can update your pull request. Good luck, I hope your PR is accepted!

@gurvirbaraich
Copy link

Okay, taken down the previous pull request and created new one #5137. Thanks @doom-goober!

@InspiredGuy InspiredGuy mentioned this issue May 1, 2023
Closed
@ratulSharker ratulSharker mentioned this issue Dec 12, 2023
Merged
@InspiredGuy
Copy link
Contributor

The fix was merged and released a while ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug effort/small good first issue p2 theme-issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@InspiredGuy @akoreman @doom-goober @gurvirbaraich
0