[cosmetic] Weather module humidity positioning #3330

crazyscot · 2024-01-02T23:02:26Z

This PR adds an option to tweak the layout of the weather module. When set, the humidity appears alongside the temperature:

khassel · 2024-01-02T23:13:35Z

Thank you for the PR.

We can only accept PR's based against our develop branch, we can sadly not enforce this technically.

So please create a new PR againt develop instead of master:

crazyscot · 2024-01-03T00:13:03Z

@khassel Sorry! I had based my branch against develop but somehow the PR came through with the wrong target. PR updated (please let me know if you need a fresh PR instead).

rejas · 2024-01-03T06:38:13Z

Also thanks from me for the PR. However I would propose to change it so that the position of the humidity is controlled via the showHumidity config parameter and not introduce a new one.

For that showHumidty would be changed to a string (with options "below" and "besideTemp" (but those names are not optimal I think)) and a deprecation warning for the boolean value.

See https://github.com/MagicMirrorOrg/MagicMirror/blob/8c0e7db494e86895a83a854cf1071c9057098ce3/modules/default/weather/weather.js#L82-88 for an ecample.

crazyscot · 2024-01-03T08:32:42Z

Also thanks from me for the PR. However I would propose to change it so that the position of the humidity is controlled via the showHumidity config parameter and not introduce a new one.

Thanks, I like that idea. I'll have a think about how to express that.

khassel · 2024-01-03T17:49:48Z

please let me know if you need a fresh PR instead

no, looks good so far (except one test)

crazyscot · 2024-01-03T20:29:57Z

OK, see what you think of this. showHumidity is now a string specifying one of a handful of possible positions.
Supported values are wind, temp, feelslike, below, false (where "false" means not displayed).
We also recognise true and false (boolean, unquoted) as legacy values and emit a deprecation warning.

(I have force-pushed this branch, I hope that the PR checks recognise that it is now based off of develop. If not I may need to redo with a fresh PR.)

Of course there will be docs to update, if we get to the point where this PR is approved then I'll send a separate PR in that repo.

khassel · 2024-01-03T21:06:09Z

test fails are unrelated to this PR, see #3291

crazyscot · 2024-01-13T02:52:10Z

Rebased back onto tip of develop

rejas

Looks good, just a minor thing.
Also, would you be able to write a test for this?

modules/default/weather/weather.js

crazyscot · 2024-01-28T05:42:55Z

Looks good, just a minor thing. Also, would you be able to write a test for this?

I've added a test now. I'm not sure how you feel about embedding a raw non-breaking-space character in the test string though? There might be a better way to do it, or maybe a tweak to the the template would leave things tidier.

rejas

nearly there, good work nonetheless!

modules/default/weather/current.njk

rejas · 2024-01-28T21:58:18Z

modules/default/weather/weather.js

@@ -14,7 +14,7 @@ Module.register("weather", {
 		updateInterval: 10 * 60 * 1000, // every 10 minutes
 		animationSpeed: 1000,
 		showFeelsLike: true,
-		showHumidity: false,
+		showHumidity: "false", // this is now a string; see current.njk


"false" as a string seems wrong. Maybe name that state "none", that is more in line with other hidden stuff (like in css).

Renamed to "none".

rejas · 2024-01-28T21:59:36Z

modules/default/weather/weather.js

@@ -80,6 +80,13 @@ Module.register("weather", {
 			Log.warn("Your are using the deprecated config values 'useBeaufort'. Please switch to windUnits!");
 			this.windUnits = "beaufort";
 		}
+		if (this.config.showHumidity === true) {


maybe do a typeof checking if its a boolean. That way you dont need to write the Log warning twice.
And then set the value like
this.config.showHumidity = this.config.showHumidity ? "wind" : "none"

Supported positions: "wind" "temp" "feelslike" "below" "false" (where "false" means not displayed). We recognise true and false (boolean, unquoted) as legacy values and emit a deprecation warning. This also gives humidity its own css class, to allow for custom styling.

crazyscot · 2024-01-29T05:11:00Z

I'm not sure how you feel about embedding a raw non-breaking-space character in the test string though?

Replaced the embedded space with its escape string \xa0.

crazyscot · 2024-01-29T05:13:21Z

nearly there, good work nonetheless!

Thanks for the feedback! I'm more at home with C++ than I am with js, it probably shows...

@bugsounet

## [2.27.0] - 2024-04-01 Thanks to: @bugsounet, @crazyscot, @illimarkangur, @jkriegshauser, @khassel, @KristjanESPERANTO, @Paranoid93, @rejas, @sdetweil and @vppencilsharpener. This release marks the first release without Michael Teeuw (@MichMich). A very special thanks to him for creating MagicMirror and leading the project for so many years. For more info, please read the following post: [A New Chapter for MagicMirror: The Community Takes the Lead](https://forum.magicmirror.builders/topic/18329/a-new-chapter-for-magicmirror-the-community-takes-the-lead). ### Added - Output of system information to the console for troubleshooting (#3328 and #3337), ignore errors under aarch64 (#3349) - [chore] Add `eslint-plugin-package-json` to lint the `package.json` files (#3368) - [weather] `showHumidity` config is now a string describing where to show this element. Supported values: "wind", "temp", "feelslike", "below", "none". (#3330) - electron-rebuild test suite for electron and 3rd party modules compatibility (#3392) - Create MM² icon and attach it to electron process (#3407) ### Updated - Update updatenotification (update_helper.js): Recode with pm2 library (#3332) - Removing lodash dependency by replacing merge by spread operator (#3339) - Use node prefix for build-in modules (#3340) - Rework logging colors (#3350) - Update pm2 to v5.3.1 with no allow-ghsas (#3364) - [chore] Update husky and let lint-staged fix ESLint issues - [chore] Update dependencies including electron to v29 (#3357) and node-ical - Update translations for estonian (#3371) - Update electron to v29 and update other dependencies - [calendar] fullDay events over several days now show the left days from the first day on and 'today' on the last day - Update layout of current weather indoor values ### Fixed - Correct apibase of weathergov weatherprovider to match documentation (#2926) - Worked around several issues in the RRULE library that were causing deleted calender events to still show, some initial and recurring events to not show, and some event times to be off an hour. (#3291) - Skip changelog requirement when running tests for dependency updates (#3320) - Display precipitation probability when it is 0% instead of blank/empty (#3345) - [newsfeed] Suppress unsightly animation cases when there are 0 or 1 active news items (#3336) - [newsfeed] Always compute the feed item URL using the same helper function (#3336) - Ignore all custom css files (#3359) - [newsfeed] Fix newsfeed stall issue introduced by #3336 (#3361) - Changed `log.debug` to `log.log` in `app.js` where logLevel is not set because config is not loaded at this time (#3353) - [calendar] deny fetch interval < 60000 and set 60000 in this case (prevent fetch loop failed) (#3382) - added message in case where config.js is missing the module.export line PR #3383 - Fixed an issue where recurring events could extend past their recurrence end date (#3393) - Don't display any `npm WARN <....>` on install (#3399) - Fixed move suncalc dependency to production from dev, as it is used by clock module - [compliments] Fix mirror not responding anymore when no compliments are to be shown (#3385) ### Deleted - Unneeded file headers (#3358) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Michael Teeuw <michael@xonaymedia.nl> Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Karsten Hassel <hassel@gmx.de> Co-authored-by: Ross Younger <crazyscot@gmail.com> Co-authored-by: Bugsounet - Cédric <github@bugsounet.fr> Co-authored-by: jkriegshauser <joshuakr@nvidia.com> Co-authored-by: illimarkangur <116028111+illimarkangur@users.noreply.github.com> Co-authored-by: sam detweiler <sdetweil@gmail.com> Co-authored-by: vppencilsharpener <tim.pray@gmail.com> Co-authored-by: Paranoid93 <6515818+Paranoid93@users.noreply.github.com>

crazyscot changed the base branch from master to develop January 3, 2024 00:11

crazyscot force-pushed the feature/weather-humidity-styling branch from 96a27c7 to bab7477 Compare January 3, 2024 20:27

crazyscot force-pushed the feature/weather-humidity-styling branch 2 times, most recently from e6d4005 to c14c583 Compare January 13, 2024 02:51

crazyscot force-pushed the feature/weather-humidity-styling branch from c14c583 to 5b01bba Compare January 14, 2024 09:35

rejas requested changes Jan 27, 2024

View reviewed changes

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

crazyscot force-pushed the feature/weather-humidity-styling branch 2 times, most recently from c4062a9 to 945bae2 Compare January 28, 2024 05:41

crazyscot requested a review from rejas January 28, 2024 05:43

rejas requested changes Jan 28, 2024

View reviewed changes

crazyscot added 3 commits January 29, 2024 18:08

PR feedback - use "none" not "false"

55e6063

replace embedded non-breaking space with \xa0

43bc2d6

crazyscot force-pushed the feature/weather-humidity-styling branch from 945bae2 to 43bc2d6 Compare January 29, 2024 05:09

crazyscot requested a review from rejas January 29, 2024 05:14

rejas approved these changes Jan 29, 2024

View reviewed changes

rejas merged commit 57de389 into MagicMirrorOrg:develop Jan 29, 2024
5 checks passed

crazyscot deleted the feature/weather-humidity-styling branch January 29, 2024 06:53

crazyscot mentioned this pull request Jan 29, 2024

weather.md: update for recent changes MagicMirrorOrg/MagicMirror-Documentation#230

Merged

rejas mentioned this pull request Apr 1, 2024

Release 2.27.0 #3410

Merged

0wenFM-01 mentioned this pull request Apr 9, 2024

Release 2.27.0 (#3410) 0wenFM-01/MagicMirrorFORK#2

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cosmetic] Weather module humidity positioning #3330

[cosmetic] Weather module humidity positioning #3330

crazyscot commented Jan 2, 2024

khassel commented Jan 2, 2024

crazyscot commented Jan 3, 2024

rejas commented Jan 3, 2024 •

edited

Loading

crazyscot commented Jan 3, 2024

khassel commented Jan 3, 2024

crazyscot commented Jan 3, 2024 •

edited

Loading

khassel commented Jan 3, 2024 •

edited

Loading

crazyscot commented Jan 13, 2024 •

edited

Loading

rejas left a comment

crazyscot commented Jan 28, 2024

rejas left a comment

rejas Jan 28, 2024

crazyscot Jan 29, 2024

rejas Jan 28, 2024

crazyscot Jan 29, 2024

crazyscot commented Jan 29, 2024

crazyscot commented Jan 29, 2024

[cosmetic] Weather module humidity positioning #3330

[cosmetic] Weather module humidity positioning #3330

Conversation

crazyscot commented Jan 2, 2024

khassel commented Jan 2, 2024

crazyscot commented Jan 3, 2024

rejas commented Jan 3, 2024 • edited Loading

crazyscot commented Jan 3, 2024

khassel commented Jan 3, 2024

crazyscot commented Jan 3, 2024 • edited Loading

khassel commented Jan 3, 2024 • edited Loading

crazyscot commented Jan 13, 2024 • edited Loading

rejas left a comment

Choose a reason for hiding this comment

crazyscot commented Jan 28, 2024

rejas left a comment

Choose a reason for hiding this comment

rejas Jan 28, 2024

Choose a reason for hiding this comment

crazyscot Jan 29, 2024

Choose a reason for hiding this comment

rejas Jan 28, 2024

Choose a reason for hiding this comment

crazyscot Jan 29, 2024

Choose a reason for hiding this comment

crazyscot commented Jan 29, 2024

crazyscot commented Jan 29, 2024

rejas commented Jan 3, 2024 •

edited

Loading

crazyscot commented Jan 3, 2024 •

edited

Loading

khassel commented Jan 3, 2024 •

edited

Loading

crazyscot commented Jan 13, 2024 •

edited

Loading