[feature] Introduce disableNextEvent to hide next sun event by bughaver · Pull Request #3769 · MagicMirrorOrg/MagicMirror

bughaver · 2025-04-24T10:17:21Z

Hello and thank you for wanting to contribute to the MagicMirror² project!

Please make sure that you have followed these 4 rules before submitting your Pull Request:

Base your pull requests against the develop branch.

Include these infos in the description:

Does the pull request solve a related issue? No

If so, can you reference the issue like this Fixes #<issue_number>? N/A

What does the pull request accomplish? Use a list if needed. Introduce showNextEvent to show/hide next sun event

If it includes major visual changes please add screenshots.

Please run npm run lint:prettier before submitting so that
style issues are fixed.

Don't forget to add an entry about your changes to
the CHANGELOG.md file.

Note: Sometimes the development moves very fast. It is highly
recommended that you update your branch of develop before creating a
pull request to send us your changes. This makes everyone's lives
easier (including yours) and helps us out on the development team.

Thanks again and have a nice day!

sdetweil · 2025-04-24T11:28:29Z

can you explain what this does?

currently the sun is shown as its next event
sunrise if its after sunset and sunset if its after sunrise

what does this new setting do?

bughaver · 2025-04-24T11:33:35Z

can you explain what this does?

currently the sun is shown as its next event

sunrise if its after sunset and sunset if its after sunrise

what does this new setting do?

This allows the option to disable the next event status on the showSunTimes. Personally I just wanted to see the sun up and sun down without the time till next sun event (like the photo).

Allows this to be configurable

bughaver · 2025-04-24T12:42:40Z

Test failing look unrelated (weather module) can the check be run again?

rejas · 2025-04-24T13:43:50Z

Will run the tests again.
Can you also add a test for this feature?

modules/default/clock/clock.js

bughaver · 2025-04-26T03:24:57Z

Will run the tests again. Can you also add a test for this feature?

added some tests

bughaver · 2025-04-26T04:27:57Z

Pr to update the docs MagicMirrorOrg/MagicMirror-Documentation#307

bughaver · 2025-04-26T04:29:53Z

modules/default/clock/clock.js

 		secondsColor: "#888888", // DEPRECATED, use CSS instead. Class "clock-second-digital" for digital clock, "clock-second" for analog clock.

 		showSunTimes: false,
+		showSunNextEvent: true,


maintaining the existing behavior

After looking at the docs PR and how showMoonTimes is configed, I liked the former approach better, sicne it resembled the behaviour of the showMoonTimes config option.
Its not uncommon to have a boolean option with more suboption that also enable it automatically when set.

But thats open for discussion.

That's fine. In my original approach I used an object to define sub options, it looks like showMoonTimes uses a single variable with a mixture of strings and booleans. should I follow the showMoonTimes approach or my original? showMoonTimes approach could be less configurable for multiple sub options if in the future someone wants to implement showSunset: false for example but understand if you want to kept it consistent. Let me know what you think is best and I can get to write it up. cc @KristjanESPERANTO

@rejas pushed an implementation similar to showMoonTimes

bughaver · 2025-04-27T05:20:00Z

@sdetweil @rejas @KristjanESPERANTO any further comments or are you happy to approve?

rejas · 2025-04-27T05:43:41Z

modules/default/clock/clock.js

 		secondsColor: "#888888", // DEPRECATED, use CSS instead. Class "clock-second-digital" for digital clock, "clock-second" for analog clock.

 		showSunTimes: false,
+		showSunNextEvent: true,


After looking at the docs PR and how showMoonTimes is configed, I liked the former approach better, sicne it resembled the behaviour of the showMoonTimes config option.
Its not uncommon to have a boolean option with more suboption that also enable it automatically when set.

But thats open for discussion.

rejas · 2025-04-27T05:45:45Z

@sdetweil @rejas @KristjanESPERANTO any further comments or are you happy to approve?

just one thing we should clarify. thanks for your patience and your docs PR.

rejas

I like this, hopefully th eothers too :-)

KristjanESPERANTO

Looks good to me 👍 🙂

bughaver · 2025-04-27T11:51:09Z

Thanks @rejas and @KristjanESPERANTO! also seeking approval on the docs changes if you have a moment MagicMirrorOrg/MagicMirror-Documentation#307

@bughaver

## [2.32.0] - 2025-07-01 Thanks to: @bughaver, @bugsounet, @khassel, @KristjanESPERANTO, @plebcity, @rejas, @sdetweil. > ⚠️ This release needs nodejs version `v22.14.0 or higher` ### Added - [config] Allow to change module order for final renderer (or dynamically with CSS): Feature `order` in config (#3762) - [clock] Added option 'disableNextEvent' to hide next sun event (#3769) - [clock] Implement short syntax for clock week (#3775) ### Changed - [refactor] Simplify module loading process (#3766) - Use `node --run` instead of `npm run` (#3764) and adapt `start:dev` script (#3773) - [workflow] Run linter and spellcheck with LTS node version (#3767) - [workflow] Split "Run test" step into two steps for more clarity (#3767) - [linter] Review linter setup (#3783) - Fix command to lint markdown in `CONTRIBUTING.md` - Re-activate JSDoc linting and fix linting issues - Refactor ESLint config to use `defineConfig` and `globalIgnores` - Replace `eslint-plugin-import` with `eslint-plugin-import-x` - Switch Stylelint config to flat format and simplify Stylelint scripts - [workflow] Replace Node.js version v23 with v24 (#3770) - [refactor] Replace deprecated constants `fs.F_OK` and `fs.R_OK` (#3789) - [refactor] Replace `ansis` with built-in function `util.styleText` (#3793) - [core] Integrate stuff from `vendor` and `fonts` folders into main `package.json`, simplifies install and maintaining dependencies (#3795, #3805) - [l10n] Complete translations (with the help of translation tools) (#3794) - [refactor] Refactored `calendarfetcherutils` in Calendar module to handle timezones better (#3806) - Removed as many of the date conversions as possible - Use `moment-timezone` when calculating recurring events, this will fix problems from the past with offsets and DST not being handled properly - Added some tests to test the behavior of the refactored methods to make sure the correct event dates are returned - [linter] Enable ESLint rule `no-console` and replace `console` with `Log` in some files (#3810) - [tests] Review and refactor translation tests (#3792) ### Fixed - [fix] Handle spellcheck issues (#3783) - [calendar] fix fullday event rrule until with timezone offset (#3781) - [feat] Add rule `no-undef` in config file validation to fix #3785 (#3786) - [fonts] Fix `roboto.css` to avoid error message `Unknown descriptor 'var(' in @font-face rule.` in firefox console (#3787) - [tests] Fix and refactor e2e test `Same keys` in `translations_spec.js` (#3809) - [tests] Fix e2e tests newsfeed and calendar to exit without open handles (#3817) ### Updated - [core] Update dependencies including electron to v36 (#3774, #3788, #3811, #3804, #3815, #3823) - [core] Update package type to `commonjs` - [logger] Review factory code part: use `switch/case` instead of `if/else if` (#3812) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Michael Teeuw <[email protected]> Co-authored-by: Kristjan ESPERANTO <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ross Younger <[email protected]> Co-authored-by: Veeck <[email protected]> Co-authored-by: Bugsounet - Cédric <[email protected]> Co-authored-by: jkriegshauser <[email protected]> Co-authored-by: illimarkangur <[email protected]> Co-authored-by: sam detweiler <[email protected]> Co-authored-by: vppencilsharpener <[email protected]> Co-authored-by: veeck <[email protected]> Co-authored-by: Paranoid93 <[email protected]> Co-authored-by: Brian O'Connor <[email protected]> Co-authored-by: WallysWellies <[email protected]> Co-authored-by: Jason Stieber <[email protected]> Co-authored-by: jargordon <[email protected]> Co-authored-by: Daniel <[email protected]> Co-authored-by: Ryan Williams <[email protected]> Co-authored-by: Panagiotis Skias <[email protected]> Co-authored-by: Marc Landis <[email protected]> Co-authored-by: HeikoGr <[email protected]> Co-authored-by: Pedro Lamas <[email protected]> Co-authored-by: veeck <[email protected]> Co-authored-by: Magnus <[email protected]> Co-authored-by: Ikko Eltociear Ashimine <[email protected]> Co-authored-by: DevIncomin <[email protected]> Co-authored-by: Nathan <[email protected]> Co-authored-by: mixasgr <[email protected]> Co-authored-by: Savvas Adamtziloglou <[email protected]> Co-authored-by: Konstantinos <[email protected]> Co-authored-by: OWL4C <[email protected]> Co-authored-by: BugHaver <[email protected]> Co-authored-by: BugHaver <[email protected]> Co-authored-by: Koen Konst <[email protected]> Co-authored-by: Koen Konst <[email protected]>

KristjanESPERANTO reviewed Apr 24, 2025

View reviewed changes

modules/default/clock/clock.js Outdated Show resolved Hide resolved

bughaver changed the title ~~[Feature] Introduce showSunNextEvent to show/hide next sun event~~ [Feature] Introduce showNextEvent to show/hide next sun event Apr 26, 2025

bughaver changed the title ~~[Feature] Introduce showNextEvent to show/hide next sun event~~ [Feature] Introduce showSunNextEvent to show/hide next sun event Apr 26, 2025

bughaver commented Apr 26, 2025

View reviewed changes

Update clock.js

8d20af5

bughaver force-pushed the showSunNextEvent branch from 598793d to 8d20af5 Compare April 26, 2025 11:59

bughaver changed the title ~~[Feature] Introduce showSunNextEvent to show/hide next sun event~~ [feature] Introduce showSunNextEvent to show/hide next sun event Apr 26, 2025

rejas requested changes Apr 27, 2025

View reviewed changes

single variable for showSunTimes sub options

b7eaa6f

bughaver force-pushed the showSunNextEvent branch from b9ab1b3 to b7eaa6f Compare April 27, 2025 06:48

rejas approved these changes Apr 27, 2025

View reviewed changes

bughaver changed the title ~~[feature] Introduce showSunNextEvent to show/hide next sun event~~ [feature] Introduce disableNextEvent to hide next sun event Apr 27, 2025

KristjanESPERANTO approved these changes Apr 27, 2025

View reviewed changes

bughaver mentioned this pull request Apr 27, 2025

Add documentation for disableNextEvent MagicMirrorOrg/MagicMirror-Documentation#307

Merged

rejas merged commit ff66829 into MagicMirrorOrg:develop Apr 27, 2025
9 checks passed

bughaver deleted the showSunNextEvent branch May 11, 2025 07:18

khassel mentioned this pull request Jun 30, 2025

Release 2.32.0 #3826

Merged

Uh oh!

Conversation

bughaver commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdetweil commented Apr 24, 2025

Uh oh!

bughaver commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bughaver commented Apr 24, 2025

Uh oh!

rejas commented Apr 24, 2025

Uh oh!

Uh oh!

bughaver commented Apr 26, 2025

Uh oh!

bughaver commented Apr 26, 2025

Uh oh!

bughaver Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rejas Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bughaver Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bughaver Apr 27, 2025

Choose a reason for hiding this comment

Uh oh!

bughaver commented Apr 27, 2025

Uh oh!

rejas Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rejas commented Apr 27, 2025

Uh oh!

rejas left a comment

Choose a reason for hiding this comment

Uh oh!

KristjanESPERANTO left a comment

Choose a reason for hiding this comment

Uh oh!

bughaver commented Apr 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bughaver commented Apr 24, 2025 •

edited

Loading

bughaver commented Apr 24, 2025 •

edited

Loading

bughaver Apr 26, 2025 •

edited

Loading

rejas Apr 27, 2025 •

edited

Loading

bughaver Apr 27, 2025 •

edited

Loading

rejas Apr 27, 2025 •

edited

Loading