Rework itemMetadataWidget

[hubsif]

Hi Yannick,

With this commit I addressed issue #678. I did quite a lot more than I had to, so here some notes:

I completely removed following code:

        // set to '' when the default defines the option but the metadata doesn't (null would be better but the API then removes it)
        if (!this.metadata.config[key] && this.defaultComponent.config[key]) this.$set(this.metadata.config, key, '')

I tried real hard to find out what you added this for, there surely must have been a reason - but couldn't.
If by !this.metadata.config[key] you meant "if that key does not exist": that doesn't make sense when iterating through the keys. If you meant "if the config value is falsy and the default truthy": I can't see how it makes sense to set the value to an empty string (or from the comment the originally desired 'null'). Perhaps you remember what it's for, if it's not just some weird leftover.

With this removed, values like the slider scale, which defaults to true, can be set to false again. Also, values now can have a 0, e.g. for the scaleSubsteps value.

The second major change is to make the preview widget dynamically update.

I then wanted to address another issue, which is that default values get reset to default when you empty its input field. Try e.g. to set another value for "Highlight Color" for cells. It defaults to blue and whenever you delete the last character (b), the value gets set to blue again.

At first I thought that comes from the many computed values. Later on I realized that this issue seems to lie in configSheet instead.
But while analyzing this, I discovered that with each keypress the many code for computed values is executed. As this seemed unnecesary, I reworked it - actually quite radically, I hope you like it.

Additionally, I streamlined the standard widget arrays and the ordered standard widget arrays into one. personalWidgets still gets computed, since like this it's updated automatically when loaded.

And I removed the viewMode, since it actually never changes. The componentSelectKey also seemed redundant.

[relativeci]

Job #21: Bundle Size — 10.88MB (~-0.01%).

fc5aab6 vs 607d560

Changed metrics (1/8)

Metric	Current	Baseline
Cache Invalidation	2.42%	0%

Changed assets by type (1/7)

	Current	Baseline
JS	8.55MB (~-0.01%)	8.55MB

---

View Job #21 report on app.relative-ci.com

[ghys]

I tried real hard to find out what you added this for, there surely must have been a reason - but couldn't.
If by !this.metadata.config[key] you meant "if that key does not exist": that doesn't make sense when iterating through the keys. If you meant "if the config value is falsy and the default truthy": I can't see how it makes sense to set the value to an empty string (or from the comment the originally desired 'null'). Perhaps you remember what it's for, if it's not just some weird leftover.

https://github.com/openhab/openhab-webui/blame/main/bundles/org.openhab.ui/web/src/components/item/metadata/item-metadata-widget.vue
It was added in #544 to fix #542 after #540.
This allows to keep the automatically-provided default widget (the metadata value is blank) and only override some of the preconfigured options. When setting it to null to clear it - like don't perform any action instead of the default one, like analyze - when posting to the REST API the server silently removed it, so it didn't work (when the config property is not set in metadata, use the automatically-provided option).

[hubsif]

don't perform any action instead of the default one

Ah ok, I actually thought it could be something like this, but seems I didn't stay on it enough.
I'll try to find a solution that fixes both, no-actions and intentionally falsy values.

[hubsif]

Ok, so I examined this further and I now know why skipped the idea this might be to override an action default value to "none": It simply doesn't work for me! 😮

When you select no action (empty option), whose resulting select option value is falsy, the config parameter gets fully deleted in configSheet, and thus in itemMetadataWidget's metadata.config. And with the parameter not being present in metadata.config, it can't be iterated over as key. Meaning: for me that line is actually never executed as intended. But this would mean that the fix never worked, which I can barely believe.

So, where's my fault? 🤔

[ghys]

You're right it doesn't seem to work (anymore?).
That's probably since I added the set-empty-config-as-null prop to config-sheet in

openhab-webui/bundles/org.openhab.ui/web/src/components/config/config-sheet.vue

Line 116 in 0a1848e

if (this.setEmptyConfigAsNull) {

...

Maybe it could be set in this case?
See #553 (comment) and #339 (comment) for more explanation on why this parameter was sometimes needed, sometimes not.

[ghys]

I struggled to try and finally have a jest test for this, like this one:
https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui/web/test/jest/__tests__/config-parameter.test.js
(which miraculously still works, and I would really really like to improve in this area)

Turns out you have to mock the Vuex store, and the SSE connection API, but you still don't have a Vue.prototype.$f7.utils in the test Vue instance, so I can't "shallow mount" a Widget Metadata Editor for testing.
Not giving up yet but it looks like a challenge.

[hubsif]

I think tests can be very helpful and are high level coding, but unfortunately setting them up and maintaining them can cost an awful lot of time 😃

I was also looking for a fix for this and now I'm struggling with YAML:
somehow this

{ config: { action: null } }

gets stringified to that:

config:
  ? action

while the other way round (with correct yaml) it works just fine 🤨

Edit:
Oh! As it turns out that ? action actually seems to be perfectly valid yaml 😬

[hubsif]

ok, so enabling setEmptyConfigAsNull and checking for a string default value makes it all work (to set no action and falsy values). Though this all seems more than a workaround than a perfectly clean solution for following reasons:

1. configSheet now sets the value to "null", just so that itemMetadataWidget can turn it into an empty string, just so that the API doesn't remove the config parameter when it shouldn't
2. the UI actually only displays the empty action option coincidentally, because parameterOptions can't match the empty string value to a select option

And of course it would be nicer to be able to display a "no action" or "none" for the empty string.

Best would be to have a consistent standard throughout the API and the UI for these things, so that all widgets could simply rely on the configuration components.

But then, ... it works 😃 . And maybe someday there'll be time to beautify things like these.

[ghys]

1. configSheet now sets the value to "null", just so that itemMetadataWidget can turn it into an empty string, just so that the API doesn't remove the config parameter when it shouldn't

👍 The option is to modify the core to allow nulls in metadata config parameters, but I don't know if it makes sense/could lead to side effects.

2. the UI actually only displays the empty action option coincidentally, because parameterOptions can't match the empty string value to a select option

¯\(ツ)/¯
That is acceptable because all non-required parameters have an empty option at the top, which would be selected if there's no matching option. I don't think it's necessary to have an explicit "no action" label.

Btw - I noticed a small bug when testing this, when you click on "Remove metadata", the dirty flag has to be cleared, because now you get the confirmation dialog... while the metadata is deleted in the background. But since it's not strictly related to this PR, it can be addressed separately.