Add missing sitemap attributes #1487

mherwege · 2022-09-01T19:48:50Z

Signed-off-by: Mark Herwege mark.herwege@telenet.be

This PR adds the possibility to configure sitemap widget parameters so far only configurable through sitemap files. It should allow fully copying sitemap files into the UI and correctly interpreting it, with the possibility to modify in the UI.

This PR depends on PR#3075

relativeci · 2022-09-01T20:15:01Z

Job #549: Bundle Size — 11.33MB (+0.07%).

35c62fe(current) vs b99ff2b main#548(baseline)

Metrics (2 changes)

	Current Job #549	Baseline Job #548
Initial JS	`1.7MB`(`-0.01%`)	`1.7MB`
Initial CSS	`607.98KB`	`607.98KB`
Cache Invalidation	`15.93%`	`41.75%`
Chunks	`218`	`218`
Assets	`242`	`242`
Modules	`1949`	`1949`
Duplicate Modules	`100`	`100`
Duplicate Code	`1.8%`	`1.8%`
Packages	`133`	`133`
Duplicate Packages	`15`	`15`

Total size by type (1 change)

	Current Job #549	Baseline Job #548
CSS	`855.93KB`	`855.93KB`
Fonts	`1.08MB`	`1.08MB`
HTML	`1.19KB`	`1.19KB`
IMG	`140.74KB`	`140.74KB`
JS	`8.98MB` (`+0.08%`)	`8.98MB`
Media	`295.6KB`	`295.6KB`
Other	`846B`	`846B`

View job #549 report View main branch activity

mherwege · 2022-09-09T07:01:30Z

PR#3075 has been merged, so this PR should now work in a recent snapshot.

ghys · 2022-09-20T17:50:53Z

Many thanks @mherwege for - finally! - implementing this much requested and desired feature.
This looks good at first sight! 👍
But I'll make a thorough test & review in the following days, so thanks for your patience.

ghys

Thanks @mherwege - here are some remarks and findings after testing:

ghys · 2022-10-05T19:34:01Z

bundles/org.openhab.ui/web/src/assets/sitemap-lexer.nearley

-MappingCommand -> %number | %identifier | %string
+Mapping -> MappingCommand _ %equals _ MappingLabel                                {% (d) => d[0][0].value.toString() + '=' + d[4][0].value.toString() %}
+MappingCommand -> %identifier | %string


Why this change?
I have a sitemap with:

Selection item=Scene_General mappings=[1="Morning",10="Cinema",11="TV",3="Bed",4="Night"]

that doesn't parse anymore:

Error: Syntax error at line 3 col 48: Selection item=Scene_General mappings=[1 ^ Unexpected number token: "1". Instead, I was expecting to see one of the following: ...

Adding quotes fixes the problem:

which generates:

Selection item=Scene_General mappings=["1"="Morning","10"="Cinema","11"="TV","3"="Bed","4"="Night"]

but since the syntax is supported (see https://www.openhab.org/docs/ui/sitemaps.html#mappings) it should parse successfully without.

This shouldn't have been removed

ghys · 2022-10-05T19:43:05Z

bundles/org.openhab.ui/web/src/components/pagedesigner/sitemap/attribute-details.vue

+    <f7-card-content v-if="attributes.length">
+      <f7-list inline-labels sortable @sortable:sort="onSort">
+        <f7-list-input v-for="(attr, idx) in attributes" :key="idx"
+                       :label="`#${idx+1}`" type="text" placeholder="command=Label" :value="attr" @input="updateAttribute(idx, $event)" clear-button />


The placeholder should probably be changed to a prop so that it can be configured in sitemap-editor.vue.

Here the placeholders are "command=Label" for all attribute editors even though it's not necessarily the correct syntax.

ghys · 2022-10-05T20:04:16Z

bundles/org.openhab.ui/web/src/components/pagedesigner/sitemap/attribute-details.vue

+<template>
+  <f7-card v-if="widget">
+    <f7-card-content v-if="attributes.length">
+      <f7-list inline-labels sortable @sortable:sort="onSort">


Nice idea to allow sorting, to enable sortable mode by default you would need:

Suggested change

<f7-list inline-labels sortable @sortable:sort="onSort">

<f7-list inline-labels sortable sortable-opposite class="sortable-enable" @sortable:sort="onSort">

The handlers display but sorting doesn't quite work well (the indexes should be recomputed after the sorting and the new order is changed afterwards...)

I actually didn't touch this code, but it indeed didn't work. It cost me some effort, but I think I figured out how to make it work. The index as a key doesn't do it, I needed to construct a key that changes when moving an item in the list.

It works great 👍

ghys

One additional comment that was left out:

ghys · 2022-10-05T20:09:00Z

bundles/org.openhab.ui/web/src/components/pagedesigner/sitemap/dslUtil.js

+            let value = v.substring(0, v.search(/[=<>]/))
+            value += v.substring(v.search(/[=<>]/)).replace(/[A-Za-z][A-Za-z0-9 _-]*/, function (x) { return '"' + x + '"' })
+            return `${value}`


When I try this:

it works properly:

Text icon="frontdoor" item=HS1DSZDoorSensor_DoorSensor label="Front Door" valuecolor=[=="CLOSED"="blue",=="OPEN"="red"]

However when changed to:

which should be valid - https://www.openhab.org/docs/ui/sitemaps.html#label-and-value-colors - "If an operator is not specified, the operator will default to ==", the generated code becomes:

Text icon="frontdoor" item=HS1DSZDoorSensor_DoorSensor label="Front Door" valuecolor=[CLOSED=""blue"",OPEN=""red""]

and parsing chokes on the repeated double quotes around the colors.

The same seems to happen when an item name is specified in the designer e.g. HS1DSZDoorSensor_LastChange=="Uninitialized" - extra double quotes are added:

Text icon="frontdoor" item=HS1DSZDoorSensor_DoorSensor label="Front Door" visibility=[HS1DSZDoorSensor_LastChange==""Uninitialized""]

In fact the examples in the sitemaps docs should probably all be tested & working.

I did one correction, and for me it now works on all examples that are in the documentation on the website. I did have to submit a small PR to correct a syntax error in one of the examples. Can you check again?

It seems there are still some cases where the quotes aren't right (the color names don't have their quotes anymore) :)
(in fact I'd like to ask, why not simply copy what the user is providing verbatim?)

outputs:

visibility=[Day_Time=="Morning",Temperature>19] labelcolor=[Temperature==22="green",Last_Update=="Uninitialized"=gray,LastUpdate=="OPEN"=red,22="green","gray"]

This was the original code, only for mapping:

} else if (key === 'mappings') { dsl += '[' const mappingsDsl = widget.config.mappings.map((m) => `${m.split('=')[0]}="${m.substring(m.indexOf('=') + 1)}"` ) dsl += mappingsDsl.join(',')

Notice it also adds quotes as they come without quotes from the widget.config. I tried generalizing for all, but that makes it more complex. Not sure if it works without these gymnastics.
Anyway, I think the missing element is to do the quoting not just once, but for all instances of strings, so just missing a 'g' in the replace regex.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege · 2022-10-07T14:29:22Z

@ghys I think I am done now. I wasn't happy on not being able to easily remove an added empty attribute (the clear button is not shown with an empty input). I now take care of it when converting to DSL and saving the sitemap, removing empty attribute lines.
All other remarks in the review should have been covered as well.

ghys · 2022-10-07T20:47:25Z

I wasn't happy on not being able to easily remove an added empty attribute (the clear button is not shown with an empty input). I now take care of it when converting to DSL and saving the sitemap, removing empty attribute lines.

Yeah that was a problem indeed. It would be better I think to try removing them altogether if the arrays are empty, not just before the save, because when you remove all items for an attribute and switch to the Code tab you get this error that you can't get rid of:

Error: Syntax error at line 9 col 92:

     Text icon="window" item=FGDW002WindowSensor_Open label="Window" visibility=[]
                                                                                 ^
Unexpected rbracket token: "]". Instead, I was expecting to see one of the following:

(to be clear it's not your fault, that was an already existing bug, but maybe it'd be nice to have it solved.)

ghys · 2022-10-07T20:51:23Z

bundles/org.openhab.ui/web/src/pages/settings/pages/sitemap/sitemap-edit.vue

 import DirtyMixin from '../../dirty-mixin'
+import { isArray } from 'zrender/lib/core/util'


This isn't right - zrender is a graphic library used in ECharts, importing it like that would screw up the webpack chunks and introduce an unwanted dependency.
You can just use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray.

Oops, indeed, that is what I meant to use. Sorry about that.

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege · 2022-10-08T08:37:09Z

Yeah that was a problem indeed. It would be better I think to try removing them altogether if the arrays are empty, not just before the save, because when you remove all items for an attribute and switch to the Code tab you get this error that you can't get rid of:
Error: Syntax error at line 9 col 92:

     Text icon="window" item=FGDW002WindowSensor_Open label="Window" visibility=[]
                                                                                 ^
Unexpected rbracket token: "]". Instead, I was expecting to see one of the following:
(to be clear it's not your fault, that was an already existing bug, but maybe it'd be nice to have it solved.)

I now don't create attributes for empty attribute arrays in DSL, but leave the empty lines until saving. That still allows the user to enter something before save. On save, the remaining empty ones will get removed. Also, if you would add the attribute in the code window, the empty lines will get replaced by the changes from the code window when switching back to the editor.

ghys

Alright LGTM now!
Closes #324 - Many thanks again for implementing this after 2 years! 🥳

jimmys01 · 2022-11-30T13:12:15Z

Great Work Thank to all that made this possible!
I have some issues.
-Commends in the file make it complain.
-At sliders the sendFrequency attribute is not recognized.
example
Slider item=RGB_middle_Speed sendFrequency=1000 switchSupport minValue=1 maxValue=40 step=2 labelcolor=[RGB_middle_Unreach==CLOSED="red"]

crcowan · 2023-08-10T19:09:52Z

I find that when the text of the sitemap has a quoted string:
Text icon="text" item=Picture_Information_CaptionAbstract label="Caption/Abstract" visibility=[Picture_Information_CaptionAbstract!="Not Available"]

That quoted string looses the quotation in the YAML:
{ "component": "Text", "config": { "icon": "text", "item": "Picture_Information_CaptionAbstract", "label": "Caption/Abstract", "visibility": [ "Picture_Information_CaptionAbstract!=Not Available" ] }

This causes the error:
2023-08-10 15:03:20.358 [WARN ] [omponents.UIComponentSitemapProvider] - Syntax error in visibility rule 'Picture_Information_CaptionAbstract!=Not Available' for widget Text

mherwege · 2023-08-21T15:15:50Z

@crcowan This should be fixed with the proposed core PR.

These parameters should be number instead of text. See: #1487 (comment) Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

…#2033) These parameters should be number instead of text. See: openhab#1487 (comment) Signed-off-by: Mark Herwege <mark.herwege@telenet.be> Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>

…#2033) These parameters should be number instead of text. See: openhab#1487 (comment) Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege requested a review from a team as a code owner September 1, 2022 19:48

ghys requested changes Oct 5, 2022

View reviewed changes

mherwege requested a review from ghys October 6, 2022 20:20

mherwege added 8 commits October 7, 2022 08:41

add missing sitemap attributes

4206706

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

linter error corrections

a11f2ef

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

further linter correction

688d2e5

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

remove unnecessary check

9444ba7

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

fix formatting

5d084d6

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

Review corrections

c7a15bb

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

Correction

1a42b9c

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

remove extra quotes

5fd3583

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege force-pushed the sitemaps branch from 8a36a40 to 5fd3583 Compare October 7, 2022 06:47

mherwege added 3 commits October 7, 2022 09:23

remove empty elements from dsl properties

d03ffdb

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

clean attributes config on save

9cd25be

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

correction

e08761f

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

ghys requested changes Oct 7, 2022

View reviewed changes

fixes

fefac41

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege requested a review from ghys October 8, 2022 08:40

ghys approved these changes Oct 8, 2022

View reviewed changes

ghys merged commit 72fe51e into openhab:main Oct 8, 2022

ghys added enhancement New feature or request main ui Main UI labels Oct 8, 2022

ghys added this to the 3.4 milestone Oct 8, 2022

mherwege deleted the sitemaps branch October 10, 2022 07:29

mherwege mentioned this pull request Aug 21, 2023

Fix sitemap visibility rule with blanks openhab/openhab-core#3769

Merged

mherwege mentioned this pull request Aug 21, 2023

Sitemap editor: Fix frequency and refresh parameters #2033

Merged

ghys pushed a commit that referenced this pull request Sep 5, 2023

[mainui] Sitemap editor fix frequency and refresh parameters (#2033)

5900864

These parameters should be number instead of text. See: #1487 (comment) Signed-off-by: Mark Herwege <mark.herwege@telenet.be>

mherwege mentioned this pull request Sep 25, 2023

Sitemap Label Colour not working #2082

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing sitemap attributes #1487

Add missing sitemap attributes #1487

mherwege commented Sep 1, 2022

relativeci bot commented Sep 1, 2022 •

edited

Loading

mherwege commented Sep 9, 2022

ghys commented Sep 20, 2022

ghys left a comment

ghys Oct 5, 2022

mherwege Oct 6, 2022

ghys Oct 5, 2022

mherwege Oct 6, 2022

ghys Oct 5, 2022

mherwege Oct 6, 2022

ghys Oct 7, 2022

ghys left a comment

ghys Oct 5, 2022

mherwege Oct 6, 2022

ghys Oct 7, 2022

mherwege Oct 8, 2022 •

edited

Loading

mherwege commented Oct 7, 2022 •

edited

Loading

ghys commented Oct 7, 2022 •

edited

Loading

ghys Oct 7, 2022

mherwege Oct 8, 2022

mherwege commented Oct 8, 2022 •

edited

Loading

ghys left a comment

jimmys01 commented Nov 30, 2022

crcowan commented Aug 10, 2023

mherwege commented Aug 21, 2023

	<f7-list inline-labels sortable @sortable:sort="onSort">
	<f7-list inline-labels sortable sortable-opposite class="sortable-enable" @sortable:sort="onSort">

		import DirtyMixin from '../../dirty-mixin'
		import { isArray } from 'zrender/lib/core/util'

Add missing sitemap attributes #1487

Add missing sitemap attributes #1487

Conversation

mherwege commented Sep 1, 2022

relativeci bot commented Sep 1, 2022 • edited Loading

Job #549: Bundle Size — 11.33MB (+0.07%).

mherwege commented Sep 9, 2022

ghys commented Sep 20, 2022

ghys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mherwege Oct 8, 2022 • edited Loading

Choose a reason for hiding this comment

mherwege commented Oct 7, 2022 • edited Loading

ghys commented Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mherwege commented Oct 8, 2022 • edited Loading

ghys left a comment

Choose a reason for hiding this comment

jimmys01 commented Nov 30, 2022

crcowan commented Aug 10, 2023

mherwege commented Aug 21, 2023

relativeci bot commented Sep 1, 2022 •

edited

Loading

mherwege Oct 8, 2022 •

edited

Loading

mherwege commented Oct 7, 2022 •

edited

Loading

ghys commented Oct 7, 2022 •

edited

Loading

mherwege commented Oct 8, 2022 •

edited

Loading