SnapDrop / snapdrop

A Progressive Web App for local file sharing
https://snapdrop.net
GNU General Public License v3.0
18.37k stars 1.68k forks source link

[bug] Dark Mode doesn't work correctly #221

Open Bellisario opened 3 years ago

Bellisario commented 3 years ago

Once changed theme you can't re-use auto change because there is localstorage setted to light or dark mode and only clear all Snapdrop data can reset it.

fm-sys commented 3 years ago

I've also noticed that. Anyway, I don't have an idea how to do it better...

abhijit-hota commented 3 years ago

I think this is the relevant behavior.

When the app/website loads for the first time, it loads with the system preferred color scheme. But if the user has changed it to a different one, that shows their preference for this site! So the app/website goes "Oh ok! I thought you liked dark mode. No worries, I'll just remember you prefer light mode for me. You can always change your preference by pressing this little button."

This is how a lot of apps work. They generally provide 3 options: System, Light and Dark and persist the one user has explicitly chosen. If not chosen, they persist the system default theme.

If I'm not wrong, this is what is happening in Snapdrop now. It doesn't provide a System option but takes care of it automatically unless the user has decided to change it.

Bellisario commented 3 years ago

I think that this can be a start: once clicked the theme button (moon button), Snapdrop shows this...

image

RobinLinus commented 3 years ago

This is a great lesson in simplicity. Even the most simple feature always turns out to be more complex in practice.

What do you guys think? Are you guys sure it is really worth to increase code & ui complexity for such a minor issue?

notangelmario commented 3 years ago

why not add a long tap so it clears out the theme localstorage variable and goes back to system default?

abhijit-hota commented 3 years ago

This is a great lesson in simplicity. Even the most simple feature always turns out to be more complex in practice.

Well said.

What do you guys think? Are you guys sure it is really worth to increase code & ui complexity for such a minor issue?

I still think that the current behavior is not buggy.

Bellisario commented 3 years ago

Thank you for your support! I have opened a new pull request to resolve this bug, but first I'd like your approvements. What do you think? Please write in the pull request!

@fm-sys @abhijit-hota @RobinLinus @notangelmario