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

Image in README is hard to read in GitHub's dark/dimmed mode #17

Closed
kevinbarabash opened this issue Apr 22, 2021 · 5 comments · Fixed by #87
Closed

Image in README is hard to read in GitHub's dark/dimmed mode #17

kevinbarabash opened this issue Apr 22, 2021 · 5 comments · Fixed by #87
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@kevinbarabash
Copy link

Screen Shot 2021-04-21 at 9 43 41 PM

Screen Shot 2021-04-21 at 9 42 36 PM

@benjaminjkraft
Copy link
Collaborator

Hah, good point! I guess I could do a white background, but that seems like it might be annoying too. Or I could use some color. Other ideas welcome.

@benjaminjkraft benjaminjkraft added the documentation Improvements or additions to documentation label Apr 22, 2021
@benjaminjkraft
Copy link
Collaborator

@kevinbarabash
Copy link
Author

SVG does support media queries so I think we could probably build an asset that reverses itself based on the browser settings. If someone has their GitHub preferences to always use dark mode regardless of the browser setting it won't handle that case.

@benjaminjkraft
Copy link
Collaborator

Oh, interesting, good point! I had switched from SVG to PNG so I didn't have to worry about fonts, but there are obviously other ways of handling that. I'll take a look when I get a chance.

@benjaminjkraft benjaminjkraft added this to the OSS "launch" milestone Sep 9, 2021
benjaminjkraft added a commit that referenced this issue Sep 9, 2021
Dark mode is, they say, the new thing, and the logo doesn't work super
well with it, because it's black on dark grey.  This is surprisingly
hard to fix.  There are a few options:
- make the logo colored; sorry, but I like black
- add a white border to the logo; this is what Apollo does but I
  think it's pretty ugly in dark mode
- add svg with media queries; this works well but only if the github
  dark mode setting matches the browser(/OS) dark mode setting, since
  that's what the media queries look at
I opted for option 3.  This required converting the text in the SVG to
paths, since it really matters that you have the exact same font.  (I
tested on Android Firefox, which empirically doesn't have the same fonts
I have on desktop.)

Fixes #17.

Issue: #17

Test plan:
tested that it looks good on both mobile firefox in dark mode, and
desktop firefox+chrome in light mode.

Reviewers: dbraley, kevinb, kevindangoor, marksandstrom
@benjaminjkraft
Copy link
Collaborator

@kevinbarabash your solution worked! Subject to the caveat you gave, which I think is better than the alternatives.

@benjaminjkraft benjaminjkraft linked a pull request Sep 9, 2021 that will close this issue
@benjaminjkraft benjaminjkraft self-assigned this Sep 9, 2021
benjaminjkraft added a commit that referenced this issue Sep 10, 2021
Dark mode is, they say, the new thing, and the logo doesn't work super
well with it, because it's black on dark grey.  This is surprisingly
hard to fix.  There are a few options:
- make the logo colored; sorry, but I like black
- add a white border to the logo; this is what Apollo does but I
  think it's pretty ugly in dark mode
- add svg with media queries; this works well but only if the github
  dark mode setting matches the browser(/OS) dark mode setting, since
  that's what the media queries look at
I opted for option 3.  This required converting the text in the SVG to
paths, since it really matters that you have the exact same font.  (I
tested on Android Firefox, which empirically doesn't have the same fonts
I have on desktop.)

Fixes #17.

Issue: #17

Test plan:
tested that it looks good on both mobile firefox in dark mode, and
desktop firefox+chrome in light mode.

Reviewers: dbraley, kevinb, kevindangoor, marksandstrom
benjaminjkraft added a commit that referenced this issue Sep 10, 2021
Dark mode is, they say, the new thing, and the logo doesn't work super
well with it, because it's black on dark grey.  This is surprisingly
hard to fix.  There are a few options:
- make the logo colored; sorry, but I like black
- add a white border to the logo; this is what Apollo does but I
  think it's pretty ugly in dark mode
- add svg with media queries; this works well but only if the github
  dark mode setting matches the browser(/OS) dark mode setting, since
  that's what the media queries look at
I opted for option 3.  This required converting the text in the SVG to
paths, since it really matters that you have the exact same font.  (I
tested on Android Firefox, which empirically doesn't have the same fonts
I have on desktop.)

Fixes #17.

Issue: #17

Test plan:
tested that it looks good on both mobile firefox in dark mode, and
desktop firefox+chrome in light mode.

Reviewers: dbraley, kevinb, kevindangoor, marksandstrom
benjaminjkraft added a commit that referenced this issue Sep 11, 2021
## Summary:
Dark mode is, they say, the new thing, and the logo doesn't work super
well with it, because it's black on dark grey.  This is surprisingly
hard to fix.  There are a few options:
- make the logo colored; sorry, but I like black
- add a white border to the logo; this is what Apollo does but I
  think it's pretty ugly in dark mode
- add svg with media queries; this works well but only if the github
  dark mode setting matches the browser(/OS) dark mode setting, since
  that's what the media queries look at
I opted for option 3.  This required converting the text in the SVG to
paths, since it really matters that you have the exact same font.  (I
tested on Android Firefox, which empirically doesn't have the same fonts
I have on desktop.)

Fixes #17.

Issue: #17

## Test plan:
tested that it looks good on both mobile firefox in dark mode, and
desktop firefox+chrome in light mode.


Author: benjaminjkraft

Reviewers: dangoor, dbraley, somewhatabstract, dnerdy, kevinbarabash

Required Reviewers: 

Approved By: dangoor, dbraley, somewhatabstract

Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint

Pull Request URL: #87
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants