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

Dark theme #220

Closed
ghost opened this issue Apr 13, 2019 · 7 comments · Fixed by #323
Closed

Dark theme #220

ghost opened this issue Apr 13, 2019 · 7 comments · Fixed by #323

Comments

@ghost
Copy link

ghost commented Apr 13, 2019

I'd love to have a dark theme for nighttime use! 😎

@evanw
Copy link
Contributor

evanw commented Aug 22, 2020

@jlfwong Are you interested in a PR for a dark theme? I'm asking first because I totally understand if you're not interested, and I don't want you to feel any pressure to accept one. I know how dark mode complicates the code by adding lots of branches, and I know how inconvenient it is to have double the testing overhead going forward. No hurt feelings if you're not interested.

I use speedscope a lot late at night and would use a dark theme if it was an option. I have also wanted to have an excuse to learn more about the speedscope code base. So I figured I'd give it a shot. It's currently on a branch here. If you are interested, I'm also happy to talk about the high-level approach first if what I did isn't a good fit. There are lots of ways to do it and I just picked something at random.

Also separately we should catch up sometime! 😄

Light theme

speedscope (light mode)

Dark theme

speedscope (dark mode)

@jlfwong
Copy link
Owner

jlfwong commented Oct 13, 2020

@evanw Sorry it took me so long to respond!

This is really cool! The high-level approach looks right to me, though I think ideally I'd like to specify functional colors by role somewhere and do something like Color.foregroundText(isDarkMode) or something along those lines to make it hard to forget to do this when adding new bits of UI in various places, rather than having isDarkMode switches everywhere. I've also seen other styling tools pass down a "theme" object via React context to avoid needing to do this at all.

When I have some more time, I'd like to understand how designers manage their color palettes around use case rather than naming the color for theming, since I think that'll apply well in this case too.

@evanw
Copy link
Contributor

evanw commented Oct 13, 2020

Makes sense. Sounds like you'd like to take it from here. Is that correct? Let me know if you need anything from me. I'm also happy to help if it's useful.

@jlfwong
Copy link
Owner

jlfwong commented Oct 13, 2020

Yep! When I have time to look into this and have things in a place I'm happy with, I'll probably send it to you to user test since it sounds like you're maybe one of the heaviest nighttime users 😂

@jlfwong
Copy link
Owner

jlfwong commented Nov 12, 2020

Okay! This is live on https://www.speedscope.app/ now, and published to npm as speedscope@1.12.0

@evanw
Copy link
Contributor

evanw commented Nov 12, 2020

Shoot I didn't catch you in time. The problem mentioned in #323 (comment) happens on https://www.speedscope.app/ for me too.

@jlfwong
Copy link
Owner

jlfwong commented Nov 12, 2020

Welp. Tracking in #327

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 a pull request may close this issue.

2 participants