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

Added css class names "today" and "tomorrow" for calendar #2939

Merged
merged 7 commits into from
Oct 11, 2022
Merged

Added css class names "today" and "tomorrow" for calendar #2939

merged 7 commits into from
Oct 11, 2022

Conversation

retroflex
Copy link
Contributor

@retroflex retroflex commented Oct 8, 2022

Added class names "today" and "tomorrow" on the calendar module tr elements (i.e. calendar items).
This way you can for example color your events today and/or tomorrow to more easily see what's happening in the near future.

Implemented by adding an event.tomorrow variable (similar to event.today) that can be used for other things in the future. Also replaced a few hardcoded values (hours, seconds etc.) with constants to make the code more consistent.

Edit: tested with normal events, split day events and events with locations.

@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2022

Codecov Report

Merging #2939 (2509ddc) into develop (a328ce5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2939   +/-   ##
========================================
  Coverage    63.82%   63.82%           
========================================
  Files            9        9           
  Lines          293      293           
========================================
  Hits           187      187           
  Misses         106      106           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rejas
Copy link
Collaborator

rejas commented Oct 9, 2022

Nice idea, thwnks. would you feel confident writing a test for this feature?

modules/default/calendar/calendar.js Outdated Show resolved Hide resolved
@retroflex
Copy link
Contributor Author

Nice idea, thwnks. would you feel confident writing a test for this feature?

@rejas I haven't written tests for MagicMirror or node/js before, so not really. I had a quick check and it seems it uses test ics files: https://github.com/MichMich/MagicMirror/tree/master/tests/configs/data
Not sure how to build an ics file containing events for today and tomorrow, as that would depend on when the test is executed. Any ideas?

@rejas
Copy link
Collaborator

rejas commented Oct 9, 2022

The trick would be here to change (mock) the date of the system when running the test. Something similar could be either in compliments/weather/calendar tests...

The last day of a multi-day spliced event did not get today or tomorrow set, fixed now.
@retroflex
Copy link
Contributor Author

@rejas after upgrading npm & node I got the tests to run with: npm run test:e2e
Is there any way to just run the calendar test?

@rejas
Copy link
Collaborator

rejas commented Oct 9, 2022

I think by adding the filename of the test behind the jest call in the nom script in package json

@retroflex
Copy link
Contributor Author

retroflex commented Oct 9, 2022

This worked fine: npm run test:e2e -- calendar_spec.js
Or to run unit tests too: npm run test -- calendar_spec.js

However when running it on my RPi3b I get this:
Error: Could not load script: "http://localhost:8080/modules/default/calendar/calendar.js

And after that this: FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory Aborted

So as much as I like tests, I'm not sure I'll get anywhere with this.

@rejas
Copy link
Collaborator

rejas commented Oct 11, 2022

Yes, adding tests here is not that easy. I opened a ticket for a general solution to this: #2942

And your PR will be merged now, thanks for the work!

@rejas rejas merged commit 85a9f14 into MagicMirrorOrg:develop Oct 11, 2022
@retroflex
Copy link
Contributor Author

Thanks s lot @rejas

rejas pushed a commit that referenced this pull request Oct 17, 2022
first PR for #2942 

- added new electron tests for calendar which test new css classes from
#2939
- moved some compliments tests from `e2e` to `electron` because of date
mocking
- removed mock stuff from compliments module
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

3 participants