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

Include a Dark Mode for extension UI #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

detunedradios
Copy link

I use Stylish heavily, primarily to develop dark themes for websites I use that do not have them. Unfortunately, it is often blinding to switch back and forth between the dark theme I am working on and Stylish's bright white interface. To that end, I've developed a Dark Mode feature for the extension.

This commit should be fairly comprehensive, though naturally there may be bugs! This introduces a new Safari extension setting for enabling dark mode (off by default), which can be toggled with a checkbox in Settings. I added a special ping to global.js for checking for Dark Mode, as well as an easily-added JavaScript file to inject the dark mode CSS if the setting is enabled. It was tricky to implement this all in such a way that was performant (as in, you don't get too many flashes of the white theme or no CSS at all before the JavaScript injects the dark theme), but I believe the solution I landed on is solid enough.

Naturally this also includes a full "Dark Mode" CSS for the entire interface of the extension, as well as dark background images and throbbers for when those are needed. Minor adjustments have also been made to buttons (which have their own separate dark-button CSS) to tone down their brightness a bit in Dark Mode.

Any feedback or suggestions would be welcome.

screen shot 2018-06-28 at 9 11 49 pm

@ondrejfuhrer
Copy link

Hey @detunedradios , this looks awesome, nice job 👍

But to be honest (not my call though), you should not restructure everything just because you like a different code style. That makes it super hard to review since there are so many changes that are completely irrelevant to what was the goal of the PR. Also it blocks (or is blocked by) any other PR since it will have merge conflicts.

The merge decision is, of course, on @350d , but personally I would suggest to revert those changes that are not relevant.

@detunedradios
Copy link
Author

detunedradios commented Jun 29, 2018

Ugh, I apologize for that - I actually agree. (Sorry, I'm a bit new to this whole Git thing.)

I was going to push another commit, but it looks like @350d is manually integrating it, so cool!

@Shawzborne
Copy link

Looks beautiful please make it happen

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