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 support #47

Merged

Conversation

CharlBest
Copy link
Contributor

Duplicate of #40 but with correct formatting and linting

As I mentioned. I don't mind if this doesn't go in, so feel free to close/disregard it if you are not interested 😀

Basic theme swap inspired by Stripe Dashboard. Initially, it was very minimal with just the svelte component. The problem is you lose what you choose if you go to dark mode. Maybe not a problem but I added a cookie to save your preference.

Default:
image

Dark:
image

@scosman scosman changed the base branch from main to extension/dark_mode April 2, 2024 04:11
@scosman scosman changed the title Dark theme selector (CI/CD checks) Dark theme support Apr 2, 2024
@scosman scosman merged commit e31fe3e into CriticalMoments:extension/dark_mode Apr 2, 2024
4 checks passed
@NDruce
Copy link

NDruce commented Apr 18, 2024

Performing replacement in whole HTML code is excessive overhead and bad way. Better make it client side only, don't store/handle anything that you can on server - it's excessive in this case too. Use localStorage instead of Cookies (they are also increasing amount of data transferred in each request) and handle this logic only on client side, server side is unnecessary for this feature.

Also Cookies code crashing app server because "you set Cookie after content was started" (smth like that error was in crash reason message)

UPD: I recoded

PR: #59

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.

4 participants