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

adding language ABAP #490 #821

Merged
merged 3 commits into from
Oct 13, 2022
Merged

adding language ABAP #490 #821

merged 3 commits into from
Oct 13, 2022

Conversation

atluft
Copy link
Contributor

@atluft atluft commented Oct 11, 2022

Based on the SAP / ABAP logo:
image

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The true color blue on a dark terminal is a little too low-contrast IMO, but the ANSI colors look good! The human eye can be weird -- sometimes something that mathematically is in the middle looks more dark or light -- but in this case the Euclidean distance is definitely closer to black, and I think it shows.

@o2sh I know this project was heavily inspired by neofetch, and AFAIK neofetch doesn't use any background colors, but do we want to implement backgrounds? A frequent issue is that some logos expect white for their negative space, and others black, and as a result the official colors sometimes only look good in a light theme or a dark theme. Otherwise contributors have to get creative with their color choices.

Also, many of our logos look better on dark themes than light. We typically prefer looking good on a dark terminal than a light one when we have to choose, right?

@atluft
Copy link
Contributor Author

atluft commented Oct 12, 2022

@spenserblack
Well said, had trouble doing the negative space for SAP. A wider version (60 chars) looks pretty good, but loses the eye at 40 chars wide.
Agree, the blue is hard to distinguish, was hoping it was just my tired old eyes. Will update.

Copy link
Collaborator

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good with both dark and light backgrounds now 👍

@spenserblack
Copy link
Collaborator

BTW @atluft you can use the Vercel preview deployment link if you want to look at what your additions/changes look like.

@o2sh
Copy link
Owner

o2sh commented Oct 12, 2022

Let's give our best for this one, this logo will be shown first when visiting onefetch.dev 😅 :

Here is a suggestion based on you design @atluft :

image

  ascii: |
    {0}xxxxxxxxxxxxxxxxxxxx
    {0}xx{1}###{0}xxx{1}##{0}xx{1}####{0}xxx{1} ##  ####   ##  ####
    {0}x{1}#{0}xxxxx{1}#{0}xx{1}#{0}x{1}#{0}xxx{1}#{0}x{1} #  # #   # #  # #   #
    {0}xx{1}###{0}xx{1}####{0}x{1}####{0}x{1}  #### ####  #### ####
    {0}xxxxx{1}#{0}x{1}#{0}xx{1}#{0}x{1}#{0}xxx{1}   #  # #   # #  # #
    {0}x{1}####{0}xx{1}#{0}xx{1}#{0}x{1}#{0}xx{1}    #  # ####  #  # #
    {0}xxxxxxxxxxxxxx
  colors:
    ansi:
      - blue
      - white
    hex:
      - '#1B387D'
      - '#FFFFFF'

@o2sh
Copy link
Owner

o2sh commented Oct 12, 2022

Also, many of our logos look better on dark themes than light. We typically prefer looking good on a dark terminal than a light one when we have to choose, right?

Yes, dark backgrounds being the de facto standard on most terminals it makes sense to do it that way. Besides with #625 in place - while playing with vercel preview -, I don't think our logos look that bad on light backgrounds 🤔

@spenserblack
Copy link
Collaborator

@o2sh ❤️ that version of the logo!

But just a heads up that that, since we don't convert true color #FFFFFF, the "ABAP" won't be easy to see on a light terminal.

I've always quietly considered --true-color never to be the compatibility mode (and preferred mode) for light terminals, though, and your suggestion should look fine with that.

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