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

Injecting darkmode.css for Messenger when appropriate #581

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Arthur-Huan
Copy link
Contributor

Pre-flight Checklist

Please ensure you've completed all of the following.

Description of Change

This is mostly a proof of concept.

Currently, none of the services display their dark themes even with the system in dark mode, Ferdium in dark mode, and the universal dark mode feature turned on. It's long bothered me that some recipes have darkmode.css files but don't seem to be injected anywhere. So I tested out making some changes to webview.js, to inject the CSS if it's detected that the system is in dark mode. It seems to work. I am proposing that the previous snippet be added to all recipes if they have a darkmode.css file.

To me, it seems this would solve the dark mode issue (assuming we have a functional darkmode.css file). But I do wish to be cautious and see what more senior and knowledgeable contributors think before I proceed.

@Arthur-Huan
Copy link
Contributor Author

Arthur-Huan commented Aug 22, 2024

Added note:

The original darkmode.css for Messenger doesn't work at all. I stole one from Instagram. It also doesn't work that well, but at least you can see some changes to show that the CSS is being injected properly during dark mode, and not injected during light mode. As a proof of concept.

It seems like most of the darkmode.css files would need changes as I've struggled finding one that works perfectly. Perhaps they're outdated as they seem to have just been copied from Franz and not updated since. That would be what I look at next, after this.

Copy link
Member

@SpecialAro SpecialAro left a comment

Choose a reason for hiding this comment

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

Hello 👋

Thank you for opening this PR!

Nevertheless, we should not be using the prefers dark mode but the internal config for the service in order to inject the dark mode.

Comment on lines +131 to +132
window.matchMedia &&
window.matchMedia('(prefers-color-scheme: dark)').matches
Copy link
Member

Choose a reason for hiding this comment

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

We should not be using this but instead use the internal config of the service for handling dark mode (I'm on mobile right now but should be something like service.isDarkMode)

Copy link
Contributor

@vraravam vraravam left a comment

Choose a reason for hiding this comment

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

same comment as @SpecialAro

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