Can't refresh oh-repeater widget using 'key:'

Hi,

Since 4.0.0M3 (and now M4 as well), oh-repeater does not refresh when using the ‘key:’ parameter.

Previously, I had been using this technique to dynamically add and remove metadata and tags and add and remove groups etc.

In plain language - I send a string to an item via my widget, then parse that string for use in rules that add and remove tags and metadata etc, then the UI usually needs to update to reflect this change.

I had been using the key: parameter to refresh my oh-repeaters with the following:

key: =Math.random() + items.LightSystem_Config.state

This still seems to work with other components, but not the oh-repeater.

Maybe @florian-h05 and @JustinG have some input on this?

I use this for all sorts of tasks.

Some of my lighting pages show this


The widget for ‘Add Associated Motion Sensors’ looks like this:

uid: lightAssociatedSensors
tags: []
props:
  parameters:
    - description: The item as a text string
      label: Item
      name: item
      required: false
    - description: Tag to search for and repeat
      label: Tag
      name: searchForTag
      required: false
      type: TEXT
  parameterGroups: []
timestamp: Dec 3, 2022, 9:27:51 PM
component: f7-page
config: {}
slots:
  default:
    - component: Label
      config:
        style:
          font-weight: bold
          margin: 16px
        text: ='CHOOSE SENSORS TO TRIGGER ' + props.item.replaceAll("_", " ")
    - component: f7-card
      config:
        style:
          border-radius: 8px
      slots:
        default:
          - component: f7-list
            slots:
              default:
                - component: oh-repeater
                  config:
                    fetchMetadata: semantics, widgetOrder
                    for: item
                    itemTags: MotionSensor
                    key: =Math.random() + items.LightSystem_Config.state
                    sourceType: itemsWithTags
                  slots:
                    default:
                      - component: oh-list-item
                        config:
                          action: command
                          actionCommand: '=
                            (loop.item.tags.includes("AssociatedWith_" + props.item)) ? props.item + "," + loop.item.name + ",REMOVEMOTION" : 
                            props.item + "," + loop.item.name + ",ADDMOTION" 
                            '
                          actionItem: LightSystem_Config
                          noChevron: true
                          title: =loop.item.name.replaceAll("MotionLightSensor_Motion", "").replaceAll("_", " ")
                        slots:
                          after:
                            - component: f7-icon
                              config:
                                color: '=(loop.item.tags.includes("AssociatedWith_" + props.item)) ? "blue" : "gray"'
                                f7: checkmark
                                visible: '=(loop.item.tags.includes("AssociatedWith_" + props.item)) ? true : false'

This toggles between sending a string of “Light Name”, “Sensor Name”, and either the string “ADDMOTION” or “REMOVEMOTION” to the string item I call “LightSystem_Config”. All seperated by “,”.

The rule then takes this and either adds or removes tags from the appropriate sensor.

console.loggerName = 'org.openhab.LightingConfig';

rules.JSRule({
  name: "Lighting config",
  description: "Execute commands from UI",
  triggers: [
    triggers.ItemStateChangeTrigger('LightSystem_Config')
  ],
  execute: (event) => {
    // console.info("Received light config command: " + items.getItem(event.itemName).state)


    const commandItem = items.getItem(event.itemName)
    const commandParts = commandItem.state.split(',')
    const lightName = commandParts[0]
    const SensorName = commandParts[1]
    const commandName = commandParts[2]
    const Sensor = items.getItem(SensorName, true)
    const assMotionSensorsGroup = items.getItem(lightName + "_AssociatedMotionSensors", true)
    
    if (commandName == "ADDMOTION") {
      console.info(commandName + " - " + SensorName + " to " + lightName)
      Sensor.addTags("AssociatedWith_" + lightName)
      Sensor.addGroups(assMotionSensorsGroup)

    } else if (commandName == "REMOVEMOTION"){
      console.info(commandName + " - " + SensorName + " from " + lightName)
      Sensor.removeTags("AssociatedWith_" + lightName)
      Sensor.removeGroups(assMotionSensorsGroup.name)
      if (assMotionSensorsGroup.members.length == 0) {
        assMotionSensorsGroup.postUpdate('NULL')
      }

    commandItem.sendCommand("waiting for next command")
  }
})

The last command that sends “waiting for next command” to the string item not only clears the text field so it looks neat in the UI, but it triggers an update of the “LightSystem_Config” item, which is required to trigger an update/change for the oh-repeater UI after the tags and groups have been applied (key: =Math.random() + items.LightSystem_Config.state)

I have not been able to test M4 yet due to upgrade issues, so I haven’t seen what impact the previous fix has had.

However, given the structure of your widget, have you tried just moving the key property up one level to the f7-list that contains the repeater? That should trigger a refresh of the list which will force the repeater to recalculate.

This is a known limitation introduced by the first fix that everything that needs to fetch data from the REST API (e.g. Items in group, Items with tags, rules with tags) does not refresh i.e. does not re-fetch that data from the REST API.
I am not sure whether you know the first reported issue, that was serious performance issues with oh-repeater on CSS visibility change and this was caused by the reload.
I fear that we have to take the compromise here, probably we can enable caching and therefore revert to the old behaviour where data was re-fetched really much.

Can you please try that?

I have tried this. It does work, but there are a few exceptions where this won’t work. If I put the button in question into an accordion, it will close. There was another exception when the layer above wasn’t suitable but I can’t remember it now. And it’s an uglier refresh, things blink about a bit more. it was perfect before.

Are you saying that this functionality has been removed on purpose? That would be annoying. This method is what has opened up OpenHAB for me to create buttons and UIs that do almost anything.

There was a bug report coming from our “main_widget” project, where we are trying to create a self configuring mobile page. To achieve this, we are heavily using the oh-repeater component with filter settings from the semantic model. We also use the visibility config option a lot within our widgets. Those visibility changes cause really bad performance issues on larger installations cause of reloading with every change. On some tests, the page was not responding for minutes.
This was the reason for the change.

That’s super annoying.

This sounds like the sort of thing that I have been struggling with. But my solution has been to be use better filters for better oh-repeater results.

My system is running super smooth, with loads of these things

If you can make a change like that, can you make a new parameter ‘refresh:’ or something?

Yeah, I’ve seen the blink issue when refreshing at a higher level than the repeater. I had hoped because, in your case, the parent element was otherwise configuration light, that the blink wouldn’t occur (at least much).

What about the possibility of just an additional repeater property that enables the original refresh strategies? Something like, if refreshTrigger: true then css changes will trigger a refresh but default behavior is the new limited strategy. That should give those few users that need this a simple fix while not impacting the Main_Widget project.

3 Likes

Finally got my upgrade issues resolved so I looked into this a little bit. PR #1919 is only a partial fix to the problem. The sample code I supplied in the issue I filed included a situation where the repeater input array changed in the context of the widget. This PR fixes the repeater refresh in this situation. It does not however, actually restore the vue key function to the repeater. The refresh is triggered by the change in the input array not by a change in the key value. You can actually completely remove the key` from the demo widget and still get the repeater to refresh.

So the original problem still stands. There are situations where the repeater input array changes outside the context of the widget and this fix doesn’t cover those situations. For my system, this is most common with metadata for items. If that metadata changes, there is no way for the repeater to register that change (and this is completely understandable) so it needs to be forced to refresh through some other mechanism.

Adding the key to the repeater’s container does cause the repeater to refresh because the container completely redraws the repeater. However the UI does flicker under this circumstance because the container also redraws so the whole repeater section is eliminated and then added back in. The repeater itself, however still does not respect the key property (well, I assume that it does actually redraw it just redraws the cached data instead of refreshing the data as well).

@florian-h05 I’m not sure I 100% follow the modifications for the new repeater data caching (or I’d just submit the PR myself), but is the solution actually just as simple as changing:

if (this.loadedSource !== null) return this.loadedSource

to

if (!this.config.key && this.loadedSource !== null) return this.loadedSource

What I currently don’t understand is when I look at the (current and old) code: How does a change to the key property trigger a refresh of the repeater? config.key is accessed nowhere and I cannot find a config binding.

That wouldn’t work, because then the loaded data would never be used. I would change the load function to return a promise and use the following if in the created method:

if (!this.config.key) this.loadData().then(…)

The computed source needs to be modified to call the load function if key is specified and the data is not cached.

I would be happy to get a PR, because then I can review myself.

That’s a really good question that I hadn’t even considered before…

Now that you mention it, it’s quite possible that it wasn’t triggering a refresh via the actual vue key attribute but was just internally causing OH to redraw the widget because of a general change to the config object.

Before I started using key in some of my widgets, I could get a similar (though not quite exactly the same) effect by just assigning a dummy property to f7 components with an expression value. For example:

- component: f7-row
  config:
    refreshThisRow: =items.some_item.state

would work to cause the row to be recalculated and redrawn when some_item changed even though refreshThisRow obviously isn’t being used for anything at all. Perhaps up until now, key was working in the same way in the repeaters.

If that’s the case, then maybe the solution is just to explicitly bind config.key to the repeater template.

Shortly, I’m going to be traveling, probably until after the 4.0 release, but if I find a chunk of time before I leave I’ll see if I can do some testing and maybe put a PR together.

That’s quite interesting, I have to admit that I have neither worked with widgets that way nor studied the widget code, but it seems very reasonable.

I’m currently away on holiday, otherwise I’d have tried this. The feature freeze is coming on Sunday 16th July, but since this is a regression/bug we can fix it till Friday 21st July (code freeze).

Would be great, because I don’t have to wait for Yannick to review then.

Thought it best to move this to the repo at this point as it’s getting fairly specific:

@hmerk what I’m proposing would require adding one property to repeaters that Main_Widget project needs cached, which I hope isn’t a problem, so please let me know what you think.

2 Likes

@JustinG I am fine with your proposal and see no issue to add a new config option to our used repeaters.

This is great to see. I don’t know what you’re talking about, but it looks like good work is being done!

FYI: Work has already been finished.

The caching is now only enabled if the new boolean parameter cacheSource is true, otherwise the repeater behaves as with 3.4.

Excellent! Yes, everything does seem to be back to normal now!

Is there any docs on cacheSource? How is it useful?

Relay good to hear!

There is the component reference (oh-repeater - Repeater | openHAB), but as MainUI says: cacheSource enables caching of the loaded source data and therefore avoids re-loading it on a page change (e.g. CSS visibility).

The main_widget project is heavily relying on oh-repeater and does not need the reload of data on page change, but suffered from severe performance issues due to all repeaters reloading really much data.

2 Likes

I will try to publish a new openHAB 4 improved version in about two weeks when I am back from holyday. There are many more improvements achieved by the help of @Nico_R

1 Like