feat: add dark theme support and format files#96
Conversation
|
I came here to ask for dark theme support. I’m glad that there’s already a PR open; however, it’s hard to review due to the formatting and linting changes (which may be one reason why it wasn’t reviewed in the past four months). Could you please revert the formatting and linting changes, and keep this PR focused on dark theme support? (In general, I don’t think opening a PR as a first-time contributor with a new linter out of the blue is a good idea: linting is a quite opinionated topic, so it’s not unlikely that the repo owner doesn’t like your idea and you worked unnecessarily. The dark theme support is less opinionated, so if you keep that part only, it’s probably easier for @aureliendavid to accept the PR.) |
|
Hello @alessandroamella Thank you for your contribution! Sorry for the late reply, I haven't had much time to focus on rsspreview lately. I agree with @jtotht that mixing the linting and the new theme makes the diff difficult to parse. I do like adding a dark theme in the way you did (using system default + options param + html class). If you could re-submit a PR with just the dark theme feature and no linting it would be easier to test and merge. (if you don't have time i'll try to isolate the theme changes myself) I try to keep this extension as simple and vanilla as I can so I'm always a bit sheepish on using extra tools and frameworks and all that (like biomejs). Thank you again for the PR, I'll try to be more responsive from now. |
|
Hi again @alessandroamella I extracted the theme changes and included them into v3.36 I credited you in the commit c775172 If that's ok with you we can close this (otherwise I can revert the changes and let you open a new PR with these changes that I can merge so you can have a named commit) thanks and sorry again for the reply delay |
|
I didn’t look into the actual code changes before due to the massive reformatting, so I didn’t realize how it works. I don’t think this checkbox solution is a good idea – it initially respects browser settings, but completely ignores them afterwards. I’m probably not the only person who uses a light theme during the day and a dark one at night – if I use the System theme Firefox theme, Firefox automatically applies the OS-level dark theme setting, even if I change it while Firefox is running, but RSSPreview doesn’t. I’d even argue the setting is unnecessary, and the theming should only use |
|
that's a good point, the classic light-dark-auto setting is probably the best option i'll try to do it when I find the time I'll open an issue so that I don't forget if I can't do it soon |
I added support for dark theme, and used Biome.js to format and lint the files