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

Add system default to auto-detect if dark or light mode is set #383

Merged
merged 4 commits into from
Oct 3, 2020
Merged

Add system default to auto-detect if dark or light mode is set #383

merged 4 commits into from
Oct 3, 2020

Conversation

06b
Copy link
Contributor

@06b 06b commented Oct 1, 2020

Add "system-default" theme which auto-detects if the a person has their system set to prefer dark or light mode and set the default theme to it.

Related to #69

@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #383 into main will decrease coverage by 2.82%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #383      +/-   ##
==========================================
- Coverage   57.86%   55.03%   -2.83%     
==========================================
  Files          23       23              
  Lines        2048     2055       +7     
  Branches      343      347       +4     
==========================================
- Hits         1185     1131      -54     
- Misses        764      818      +54     
- Partials       99      106       +7     
Impacted Files Coverage Δ
js/user-preferences.js 92.59% <ø> (ø)
js/themes.js 100.00% <100.00%> (ø)
js/classes/FlexibleMonthCalendar.js 54.37% <0.00%> (-7.70%) ⬇️
js/classes/FixedDayCalendar.js 91.89% <0.00%> (-6.09%) ⬇️
js/classes/Calendar.js 68.57% <0.00%> (-5.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d719068...68a9488. Read the comment docs.

@araujoarthur0
Copy link
Collaborator

Thanks @06b! Why do you consider it experimental?
Also, could you please add a gif showing how this works based on your system's color? In which OS did you test it?

@araujoarthur0
Copy link
Collaborator

Please also add a commit adding this to the test at time-to-leave_tests__renderer_\themes.js :)

@06b
Copy link
Contributor Author

06b commented Oct 2, 2020

Hi @araujoarthur0

I felt that it was experimental as I was only able to test it on Windows (Windows 10 Pro, Version 2004, OS build 19041.508) and since Time to Leave works on MacOS & Linux I thought that perhaps it should at least be tested on other operating systems before considering if it's okay not to consider it experimental.

I also wasn't sure if the theme name could be considered misleading, for example if someone used a high contrast theme would they expect that it should pickup the high contrast or if they picked a certain accent color on their computer, would they expect that Time to Leave would use that since the theme was "system-default"

So I figured if I included (experimental) - that hopefully users might be understanding that if they assumed something such as autodetecting high contrast mode / not using accent colors from their system and it didn't work... and maybe someone might think of a better name (but I could also be overthinking this, naming things is hard)

I will add a gif showing how it works shortly.

@06b
Copy link
Contributor Author

06b commented Oct 2, 2020

Attached gif, starts with operating system default to dark mode, open up time to start and then it's on dark mode. Closed out of time to start, set operating system default to light mode, open up time to start and it's on light mode. Ends with keeping time to start open, and switching the operating system from light to dark mode, which in this case - I must go to view > reload.

TimeToLeave

While I don't think it would be common for a user to change the default app mode as shown in the gif often, it is possible that there might be operating systems or applications which would switch the mode depending on time of day, etc for example.

@thamara
Copy link
Collaborator

thamara commented Oct 2, 2020

Wow! This looks really cool!
I have tested this in MacOS and it works as well!

My-Movie

But something I noticed is that the current width of the preferences window is not enough and the text becomes a bit clutter:

image

Can you please increase the initial size of the preferences window to better fit the content? For reference, on MacOS the minimal width should be 440px.

src/preferences.html Outdated Show resolved Hide resolved
@araujoarthur0
Copy link
Collaborator

araujoarthur0 commented Oct 2, 2020

This is looking really neat! Gives off a very professional feeling hahaha

Co-authored-by: Thamara Andrade <tkcandrade@gmail.com>
Copy link
Collaborator

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Really awesome work!

@thamara thamara merged commit 03bf7eb into TTLApp:main Oct 3, 2020
@thamara
Copy link
Collaborator

thamara commented Oct 3, 2020

Congratulations on your first merged PR on TTL!

Here's to many more to come!

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