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

Adding dark mode #15

Merged
merged 26 commits into from
Jun 20, 2021
Merged

Adding dark mode #15

merged 26 commits into from
Jun 20, 2021

Conversation

voruti
Copy link
Contributor

@voruti voruti commented Jun 8, 2021

Closes #14

Btw: The nodejs.yml workflow seems to be broken ...

@voruti voruti marked this pull request as ready for review June 8, 2021 17:00
@voruti
Copy link
Contributor Author

voruti commented Jun 8, 2021

The missing issues of the "Codacy Static Code Analysis"-check are senseless:

  1. "selector-type-no-unknown": Okay, fine, but they are correct. Maybe add them to a list of "known" tags?
  2. "no-descending-specificity": These have no influence on the resulting style, but I ordered them like this to keep them in categories (e.g. chat selection, chat history, settings, profile, etc.). Maybe ignore these checks?

What to do now?

@voruti
Copy link
Contributor Author

voruti commented Jun 8, 2021

Btw: The nodejs.yml workflow seems to be broken ...

Fixed in #16

@callFEELD
Copy link
Owner

Heya, thank you for your work so far. Appreciate it!

I also partly worked on dark mode (didn't push it). But since some people prefer dark mode and some light mode I would suggest adding an option to switch between both designs.

But I am not sure how the settings options should be integrated tbh.

Maybe you've got an idea.

Copy link
Owner

@callFEELD callFEELD left a comment

Choose a reason for hiding this comment

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

Quick review point for the CSS:

Could you use CSS variables for the color values. Makes changing the color values easier.

@voruti
Copy link
Contributor Author

voruti commented Jun 10, 2021

But I am not sure how the settings options should be integrated tbh.

I also thought about that and simplest solution would be to adopt the operating system settings (that's how I already implemented it - these CSS rules are only activated if the OS is set to dark mode).

Another idea I had, was to add a new context menu item to the tray icon, which would toggle the dark mode on and off.
This gives rise to some questions: What's the default value? How to save this setting/toggle across app restarts? How to implement this into/with electron? (I never worked with electron before) ...

@callFEELD
Copy link
Owner

The missing issues of the "Codacy Static Code Analysis"-check are senseless:

1. "selector-type-no-unknown": Okay, fine, but they are correct. Maybe add them to a list of "known" tags?

2. "no-descending-specificity": These have no influence on the resulting style, but I ordered them like this to keep them in categories (e.g. chat selection, chat history, settings, profile, etc.). Maybe ignore these checks?

What to do now?

Removed the senseless checks: https://app.codacy.com/gh/callFEELD/Threema-Desktop/pullRequest?prid=7565215

Next commits will probably pass the test.

@voruti voruti requested a review from callFEELD June 13, 2021 12:42
Copy link
Owner

@callFEELD callFEELD left a comment

Choose a reason for hiding this comment

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

Everything looks good!

@callFEELD callFEELD merged commit b7f733f into callFEELD:master Jun 20, 2021
@voruti voruti deleted the adding-dark-mode branch June 23, 2021 16:24
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.

Enhancement: Adding a dark mode
2 participants