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

Fix #232 - Automatically advance internal date on day change #269

Closed
wants to merge 1 commit into from

Conversation

tupaschoal
Copy link
Collaborator

Related issue

#232

Context / Background

We lost the feature of automatically transitioning the day when the user leaves TTL open and the day changes when we stopped redrawing everything from scratch.

What change is being introduced by this PR?

We now skip again to the current date when the day changes

How will this be tested?

We'll know at midnight :D

@tupaschoal
Copy link
Collaborator Author

@araujoarthur0 what do you think?

@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #269 into master will increase coverage by 1.70%.
The diff coverage is 15.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
+ Coverage   55.08%   56.78%   +1.70%     
==========================================
  Files          13       13              
  Lines        1220     1231      +11     
  Branches      218      220       +2     
==========================================
+ Hits          672      699      +27     
+ Misses        480      466      -14     
+ Partials       68       66       -2     
Impacted Files Coverage Δ
js/calendar.js 0.00% <0.00%> (ø)
js/classes/Calendar.js 80.32% <0.00%> (-0.29%) ⬇️
main.js 0.00% <0.00%> (ø)
src/preferences.js 0.00% <0.00%> (ø)
js/user-preferences.js 92.23% <60.00%> (+8.39%) ⬆️
js/window-aux.js 100.00% <0.00%> (+100.00%) ⬆️

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 1bf9624...c79284a. Read the comment docs.

@araujoarthur0
Copy link
Collaborator

I did something similar yesterday and was waiting to check today 🤣

But I chose to only change the day if the internal _today variable was set to yesterday.
That way if the user left the program on an older day/month, he won't wake up with the program changed.

@tupaschoal
Copy link
Collaborator Author

hahahah yeah, I thought about this scenario and was just lazy. Just open your PR then, if it works and I'll close mine

@tupaschoal tupaschoal closed this Jun 19, 2020
@tupaschoal tupaschoal deleted the auto-advance-day branch June 19, 2020 16:59
@araujoarthur0
Copy link
Collaborator

araujoarthur0 commented Jun 19, 2020

Check out this commit: araujoarthur0@a9ea9be

@tupaschoal
Copy link
Collaborator Author

I did and laid out some comments there. If it works, just open a PR and let's resume it there :D

@araujoarthur0
Copy link
Collaborator

Great!
I just thought of something, actually. We don't really update the internal _today variable anywhere. And in my commit I'm not changing it if the person is not looking at the day. This would cause an issue if the day change switches the month, and the person clicks on "go to current day". The Day Calendar doesn't have this issue because it constantly updates the _today variable.

Perhaps we need to change a little the concept of "today" and "calendar day" to fix more than just the refresh.

@tupaschoal
Copy link
Collaborator Author

Hmm, correct, I thought that _goToCurrentDate would always update the whole date. Maybe we should?

@araujoarthur0
Copy link
Collaborator

Yes! I changed the internal variable idea as well, in #272

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.

None yet

2 participants