Merge pull request #2103 from rejas/issue_1985

Fix "undefined" in weather modules header.
MagicMirrorOrg · Sep 2, 2020 · 8427a9f · 8427a9f
2 parents 904f5a2 + 16c5eba
commit 8427a9f
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,12 +30,13 @@ _This release is scheduled to be released on 2020-10-01._
 
 ### Fixed
 
-- Fix backward compatibility issues for Safari < 11. [#1985](/~https://github.com/MichMich/MagicMirror/issues/1985)
+- Fix backward compatibility issues for Safari < 11.
 - Fix the use of "maxNumberOfDays" in the module "weatherforecast depending on the endpoint (forecast/daily or forecast)". [#2018](/~https://github.com/MichMich/MagicMirror/issues/2018)
 - Fix calendar display. Account for current timezone. [#2068](/~https://github.com/MichMich/MagicMirror/issues/2068)
 - Fix logLevel being set before loading config.
 - Fix incorrect namespace links in svg clockfaces. [#2072](/~https://github.com/MichMich/MagicMirror/issues/2072)
-- Fix weather/providers/weathergov for API guidelines [#2045]
+- Fix weather/providers/weathergov for API guidelines. [#2045](/~https://github.com/MichMich/MagicMirror/issues/2045)
+- Fix "undefined" in weather modules header. [#1985](/~https://github.com/MichMich/MagicMirror/issues/1985)
 
 ## [2.12.0] - 2020-07-01
 

diff --git a/js/main.js b/js/main.js
@@ -42,6 +42,8 @@ var MM = (function () {
 
 			if (typeof module.getHeader() === "undefined" || module.getHeader() !== "") {
 				moduleHeader.style.display = "none;";
+			} else {
+				moduleHeader.style.display = "block;";
 			}
 
 			var moduleContent = document.createElement("div");
@@ -218,7 +220,7 @@ var MM = (function () {
 
 		headerWrapper[0].innerHTML = newHeader;
 		if (headerWrapper.length > 0 && newHeader) {
-			delete headerWrapper[0].style;
+			headerWrapper[0].style.display = "block";
 		} else {
 			headerWrapper[0].style.display = "none";
 		}

diff --git a/modules/default/currentweather/currentweather.js b/modules/default/currentweather/currentweather.js
@@ -37,6 +37,8 @@ Module.register("currentweather", {
 		weatherEndpoint: "weather",
 
 		appendLocationNameToHeader: true,
+		useLocationAsHeader: false,
+
 		calendarClass: "calendar",
 		tableClass: "large",
 
@@ -267,15 +269,16 @@ Module.register("currentweather", {
 
 	// Override getHeader method.
 	getHeader: function () {
-		if (this.config.appendLocationNameToHeader && this.data.header !== undefined) {
-			return this.data.header + " " + this.fetchedLocationName;
-		}
-
 		if (this.config.useLocationAsHeader && this.config.location !== false) {
 			return this.config.location;
 		}
 
-		return this.data.header;
+		if (this.config.appendLocationNameToHeader) {
+			if (this.data.header) return this.data.header + " " + this.fetchedLocationName;
+			else return this.fetchedLocationName;
+		}
+
+		return this.data.header ? this.data.header : "";
 	},
 
 	// Override notification handler.

diff --git a/modules/default/weather/README.md b/modules/default/weather/README.md
@@ -1,5 +1,5 @@
 # Weather Module
 
-This module aims to be the replacement for the current `currentweather` and `weatherforcast` modules. The module will be configurable to be used as a current weather view, or to show the forecast. This way the module can be used twice to fullfil both purposes.
+This module aims to be the replacement for the current `currentweather` and `weatherforcast` modules. The module will be configurable to be used as a current weather view, or to show the forecast. This way the module can be used twice to fulfill both purposes.
 
 For configuration options, please check the [MagicMirror² documentation](https://docs.magicmirror.builders/modules/weather.html).
diff --git a/modules/default/weather/weather.js b/modules/default/weather/weather.js
@@ -73,11 +73,12 @@ Module.register("weather", {
 
 	// Override getHeader method.
 	getHeader: function () {
-		if (this.config.appendLocationNameToHeader && this.data.header !== undefined && this.weatherProvider) {
-			return this.data.header + " " + this.weatherProvider.fetchedLocation();
+		if (this.config.appendLocationNameToHeader && this.weatherProvider) {
+			if (this.data.header) return this.data.header + " " + this.weatherProvider.fetchedLocation();
+			else return this.weatherProvider.fetchedLocation();
 		}
 
-		return this.data.header;
+		return this.data.header ? this.data.header : "";
 	},
 
 	// Start the weather module.

diff --git a/modules/default/weatherforecast/weatherforecast.js b/modules/default/weatherforecast/weatherforecast.js
@@ -103,7 +103,7 @@ Module.register("weatherforecast", {
 	getDom: function () {
 		var wrapper = document.createElement("div");
 
-		if (this.config.appid === "") {
+		if (this.config.appid === "" || this.config.appid === "YOUR_OPENWEATHER_API_KEY") {
 			wrapper.innerHTML = "Please set the correct openweather <i>appid</i> in the config for module: " + this.name + ".";
 			wrapper.className = "dimmed light small";
 			return wrapper;
@@ -206,10 +206,11 @@ Module.register("weatherforecast", {
 	// Override getHeader method.
 	getHeader: function () {
 		if (this.config.appendLocationNameToHeader) {
-			return this.data.header + " " + this.fetchedLocationName;
+			if (this.data.header) return this.data.header + " " + this.fetchedLocationName;
+			else return this.fetchedLocationName;
 		}
 
-		return this.data.header;
+		return this.data.header ? this.data.header : "";
 	},
 
 	// Override notification handler.

diff --git a/tests/configs/modules/display.js b/tests/configs/modules/display.js
@@ -0,0 +1,42 @@
+/* Magic Mirror Test config for display setters module using the helloworld module
+ *
+ * MIT Licensed.
+ */
+var config = {
+	port: 8080,
+	ipWhitelist: ["127.0.0.1", "::ffff:127.0.0.1", "::1"],
+
+	language: "en",
+	timeFormat: 24,
+	units: "metric",
+	electronOptions: {
+		fullscreen: false,
+		width: 800,
+		height: 600,
+		webPreferences: {
+			nodeIntegration: true
+		}
+	},
+
+	modules: [
+		{
+			module: "helloworld",
+			position: "top_bar",
+			header: "test_header",
+			config: {
+				text: "Test Display Header"
+			}
+		},
+		{
+			module: "helloworld",
+			position: "bottom_bar",
+			config: {
+				text: "Test Hide Header"
+			}
+		}
+	]
+};
+/*************** DO NOT EDIT THE LINE BELOW ***************/
+if (typeof module !== "undefined") {
+	module.exports = config;
+}
diff --git a/tests/configs/modules/positions.js b/tests/configs/modules/positions.js
@@ -20,7 +20,7 @@ var config = {
 	},
 
 	modules:
-		// Using exotic content. This is why dont accept go to JSON configuration file
+		// Using exotic content. This is why don't accept go to JSON configuration file
 		(function () {
 			var positions = ["top_bar", "top_left", "top_center", "top_right", "upper_third", "middle_center", "lower_third", "bottom_left", "bottom_center", "bottom_right", "bottom_bar", "fullscreen_above", "fullscreen_below"];
 			var modules = Array();

diff --git a/tests/e2e/modules_display_spec.js b/tests/e2e/modules_display_spec.js
@@ -0,0 +1,41 @@
+const helpers = require("./global-setup");
+
+const describe = global.describe;
+const it = global.it;
+
+describe("Display of modules", function () {
+	helpers.setupTimeout(this);
+
+	var app = null;
+
+	beforeEach(function () {
+		return helpers
+			.startApplication({
+				args: ["js/electron.js"]
+			})
+			.then(function (startedApp) {
+				app = startedApp;
+			});
+	});
+
+	afterEach(function () {
+		return helpers.stopApplication(app);
+	});
+
+	describe("Using helloworld", function () {
+		before(function () {
+			// Set config sample for use in test
+			process.env.MM_CONFIG_FILE = "tests/configs/modules/display.js";
+		});
+
+		it("should show the test header", async () => {
+			await app.client.waitForExist("#module_0_helloworld", 10000);
+			return app.client.element("#module_0_helloworld .module-header").isVisible().should.eventually.equal(true).getText("#module_0_helloworld .module-header").should.eventually.equal("TEST_HEADER");
+		});
+
+		it("should show no header if no header text is specified", async () => {
+			await app.client.waitForExist("#module_1_helloworld", 10000);
+			return app.client.element("#module_1_helloworld .module-header").isVisible().should.eventually.equal(false);
+		});
+	});
+});