Make use of eclipse/smarthome#4291 #386

mueller-ma · 2017-09-30T18:02:54Z

This commit enhances charts to support themes, scale legend according to
dpi and create option to hide legend

Signed-off-by: mueller-ma mueller-ma@users.noreply.github.com

This commit enhances charts to support themes, scale legend according to dpi and create option to hide legend Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

hreichert · 2017-10-01T14:22:07Z

@mueller-ma That's pretty cool!

Chart themes

👍 Works good.

One note: As the default BasicUI uses the bright theme, I suggest to use the bright theme also for the HABDroid.Basic_ui theme.

Chart DPI

👍 Works good.

Screenshot:

Chart legend

I'm not sure about the new setting for "Show chart legend".
The user can already set the legend=true/false attribute in his sitemap definition for the Chart element. But this gets ignored now. Maybe it's better to extend the OpenHABWidget.java and OpenHAB2Widget.java classes for this new sitemap attribute and take this setting to construct the chart URL. (refer to the 'period' attribute)

Here is a screenshot of the widgetJson from a sitemap with Chart with legend=false:

Note: It's a Boolean

Thanks for your work!

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

mueller-ma · 2017-10-01T18:55:25Z

@hreichert I didn't know, that the legend attribute can be set in the sitemap.
Please test again. Also check what happens if no legend setting is set. The default should be true?

mueller-ma · 2017-10-01T18:57:06Z

One note: As the default BasicUI uses the bright theme, I suggest to use the bright theme also for the HABDroid.Basic_ui theme.

Since this is my personal opinion that white looks better than bright, please take two screenshots and post it here and maybe in the forum and make a poll.

hreichert · 2017-10-01T19:18:06Z

@mueller-ma

Please see inline comments.

If the legend parameter is not set for a Chart item in the sitemap, the following is the default:

Legend is automatically hidden, if there is only one chart series (-> one item) - otherwise the legend is shown

BasicUI theme: In my opinion, white looks also better and more modern. Right now I can not take screenshots, because my OH runs currently on the official snapshot distro, and the white chart theme is not yet integrated.

hreichert · 2017-10-01T21:18:36Z

mobile/src/main/java/org/openhab/habdroid/model/OpenHAB2Widget.java

@@ -64,6 +64,8 @@ private OpenHAB2Widget(OpenHABWidget parent, JSONObject widgetJson, String iconF
                this.setPeriod(widgetJson.getString("period"));
            if (widgetJson.has("service"))
                this.setService(widgetJson.getString("service"));
+            if (widgetJson.has("legend"))
+                this.setService(widgetJson.getString("legend"));


Wrong setter called

hreichert · 2017-10-01T21:21:28Z

mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java

@@ -25,7 +25,8 @@
 	private String type;
 	private String url;
 	private String period = "";
-    private String service = "";
+	private String service = "";
+	private boolean legend = true;


I suggest a Boolean with default null here, so that the behaviour is similiar to the other UI providers:
If the user does not set a legend attribute, it is not delivered in the REST API, and this should stay null.
In the OpenHABWidgetAdapter, check if this is not-null, and only append the `legend´ parameter to the chart URL if this is non-null.
(See comment in PR regarding default handling)
This gives the following behaviour:

User defines legend with true/false: legend parameter is appended to URL, respect users wish

User does NOT define legend: do not append legend parameter -> the OH ChartProvider takes the default (show legend if more than one item)

I was aware of this problem, but couldn't find a good solution. Thanks!

hreichert · 2017-10-01T21:45:32Z

mobile/src/main/java/org/openhab/habdroid/model/OpenHABWidget.java

@@ -192,6 +193,10 @@ public void setPeriod(String period) {

    public void setService(String service) { this.service = service; }

+	public boolean getLegend() { return legend; }


See above: Boolean

hreichert · 2017-10-01T21:46:11Z

mobile/src/main/java/org/openhab/habdroid/ui/OpenHABWidgetAdapter.java

+                chartUrl += "&dpi=" + dpi;
+
+                // add legend
+                chartUrl += "&legend=" + openHABWidget.getLegend();


Only add legend parameter to URL if openHABWidget.getLegend() != null; see above

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

hreichert · 2017-10-03T18:45:20Z

@mueller-ma Perfect, everything works as expected! Thanks again for your efforts.
FYI: Here is the PR for the white theme and some other optical improvements: eclipse-archived/smarthome#4368

mueller-ma · 2017-10-04T15:31:10Z

@hreichert Does the background of the charts always match the background of the app? Do you think a theme with a transparent background and bright font would be a good addition?

hreichert · 2017-10-04T15:41:16Z

@mueller-ma The current themes do match the colors from the "Hell", "Dunkel" and "Schwarz/AMOLED" app themes (I have currently only the german theme names available).

A transparent theme is, among other things, on my todo list for future chart improvements.
But for now I do not want to amend the pending PR, as Kai Kreuzer already approved the changes.

digitaldan · 2017-10-08T22:36:09Z

lgtm

Make use of eclipse-archived/smarthome#4291

cac1c0b

This commit enhances charts to support themes, scale legend according to dpi and create option to hide legend Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

mueller-ma mentioned this pull request Sep 30, 2017

Decrease chart resolution so text size is increased #373

Closed

Read legend setting from oh server

3887b72

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

hreichert reviewed Oct 2, 2017

View reviewed changes

Use Boolean

1ee6bef

Signed-off-by: mueller-ma <mueller-ma@users.noreply.github.com>

Merge branch 'master' into 383-charts

beecd63

digitaldan merged commit b1a1991 into openhab:master Oct 8, 2017

mueller-ma deleted the 383-charts branch October 9, 2017 16:24

mueller-ma mentioned this pull request Oct 9, 2017

Charts are not themed #383

Closed

mueller-ma added the enhancement Indicates new feature requests label Dec 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make use of eclipse/smarthome#4291 #386

Make use of eclipse/smarthome#4291 #386

mueller-ma commented Sep 30, 2017

hreichert commented Oct 1, 2017 •

edited

mueller-ma commented Oct 1, 2017

mueller-ma commented Oct 1, 2017

hreichert commented Oct 1, 2017 •

edited

hreichert Oct 1, 2017

hreichert Oct 1, 2017

mueller-ma Oct 2, 2017

hreichert Oct 1, 2017

hreichert Oct 1, 2017

hreichert commented Oct 3, 2017

mueller-ma commented Oct 4, 2017

hreichert commented Oct 4, 2017

digitaldan commented Oct 8, 2017

		@@ -192,6 +193,10 @@ public void setPeriod(String period) {

		public void setService(String service) { this.service = service; }

		public boolean getLegend() { return legend; }

Make use of eclipse/smarthome#4291 #386

Make use of eclipse/smarthome#4291 #386

Conversation

mueller-ma commented Sep 30, 2017

hreichert commented Oct 1, 2017 • edited

Chart themes

Chart DPI

Chart legend

mueller-ma commented Oct 1, 2017

mueller-ma commented Oct 1, 2017

hreichert commented Oct 1, 2017 • edited

hreichert Oct 1, 2017

Choose a reason for hiding this comment

hreichert Oct 1, 2017

Choose a reason for hiding this comment

mueller-ma Oct 2, 2017

Choose a reason for hiding this comment

hreichert Oct 1, 2017

Choose a reason for hiding this comment

hreichert Oct 1, 2017

Choose a reason for hiding this comment

hreichert commented Oct 3, 2017

mueller-ma commented Oct 4, 2017

hreichert commented Oct 4, 2017

digitaldan commented Oct 8, 2017

hreichert commented Oct 1, 2017 •

edited

hreichert commented Oct 1, 2017 •

edited