Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widget variables not accessible to custom sub-widgets #2437

Closed
SimonB27P opened this issue Mar 1, 2024 · 10 comments · Fixed by #2533
Closed

Widget variables not accessible to custom sub-widgets #2437

SimonB27P opened this issue Mar 1, 2024 · 10 comments · Fixed by #2533
Labels
bug Something isn't working main ui Main UI

Comments

@SimonB27P
Copy link

The problem

In a custom widget, variables can be manipulated using actions. Those variables can be used in components in the widget.

However in a custom sub-widget, the variables are not accessible. Any attempt to change a variable in the custom sub-widget is not reflected in the variable in the parent widget. Similarly, variables such as loop.name[index] or loop.name_source are not available in the custom sub-widget

Expected behavior

It would be expected / desirable to have full access to the variables in the parent widget in the scope of the custom sub-widget. If an oh-toggle, for example, were to be replaced with a custom toggle, it would be expected that the custom toggle would be able to manipulate a parent widget variable in the same way the oh-toggle can. Values can be passed into the sub-widget using props but the sub-widget should be able to manipulate the variable in the parent's scope in the same way that an oh-component can.

If possible it would be desirable to be able to pass variables to sub-widgets either by value or by reference rather than have to update the code of the sub-widget to match the name of the parent widget variable. That would make a library of sub-widgets more practical to use.

Steps to reproduce

  1. Create a parent widget that has components modifying various variables
  2. Create an accordion list using an itemsInGroup sourcetype
  3. Within the list use a custom widget that is intended to modify the variables in the parent widget

Neither the parent widget's variables nor the loop.name_source variable of the list are available in the custom widget. Any attempt to manipulate the variable seems only to manipulate a local copy and does not affect the variable in the parent widget.

Your environment

runtimeInfo:
  version: 4.1.1
  buildString: Release Build
locale: en-GB
systemInfo:
  configFolder: /etc/openhab
  userdataFolder: /var/lib/openhab
  logFolder: /var/log/openhab
  javaVersion: 17.0.9
  javaVendor: Raspbian
  osName: Linux
  osVersion: 6.1.21-v8+
  osArchitecture: arm
  availableProcessors: 4
  freeMemory: 167726040
  totalMemory: 382373888
  uptime: 2336371
  startLevel: 100
addons:
  - automation-jsscripting
  - binding-enocean
  - binding-homematic
  - binding-hue
  - binding-samsungtv
  - binding-squeezebox
  - binding-zwave
  - misc-hueemulation
  - misc-openhabcloud
  - persistence-rrd4j
  - transformation-map
  - ui-basic
clientInfo:
  device:
    ios: false
    android: false
    androidChrome: false
    desktop: true
    iphone: false
    ipod: false
    ipad: false
    edge: false
    ie: false
    firefox: true
    macos: false
    windows: true
    cordova: false
    phonegap: false
    electron: false
    nwjs: false
    webView: false
    webview: false
    standalone: false
    os: windows
    pixelRatio: 1
    prefersColorScheme: dark
  isSecureContext: false
  locationbarVisible: true
  menubarVisible: true
  navigator:
    cookieEnabled: true
    deviceMemory: N/A
    hardwareConcurrency: 4
    language: en-GB
    languages:
      - en-GB
      - en
    onLine: true
    platform: Win32
  screen:
    width: 1280
    height: 1024
    colorDepth: 24
  support:
    touch: false
    pointerEvents: true
    observer: true
    passiveListener: true
    gestures: false
    intersectionObserver: true
  themeOptions:
    dark: dark
    filled: true
    pageTransitionAnimation: default
    bars: light
    homeNavbar: default
    homeBackground: default
    expandableCardAnimation: default
  userAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:122.0) Gecko/20100101 Firefox/122.0
timestamp: 2024-03-01T12:41:06.234Z

Browser console

Browser network traffic

Additional information

@SimonB27P SimonB27P added bug Something isn't working main ui Main UI labels Mar 1, 2024
@ghys
Copy link
Member

ghys commented Mar 1, 2024

I explained (partly) how propagation works with personal widgets when I introduced the variables here:
#356 (comment)

On the illustration there you can see how it's not desirable for the two personal widgets at the bottom to manipulate the same "inputText" variable. However, if a variable is already defined at the parent's level then it will be copied to the sub-widget's context - not passed by reference (I think):

if (this.context.vars) {
for (const varKey in this.context.vars) {
this.$set(this.widgetVars, varKey, this.context.vars[varKey])
}
}

As for the loop object, these are read-only anyway so you can pass the values you need from this object as props. When you design a personal widget in general I don't think you want it to depend on its parent's context.

@JustinGeorgi
Copy link
Contributor

When you design a personal widget in general I don't think you want it to depend on its parent's context.

The problem here seems to be, however, that the asymmetry between the behavior of child <-> parent propagation based only on whether the child is defined as a separate widget or defined in-line in the parent code is counter-intuitive to a lot of users.

Propagaton From Parent To Parent
In-line Yes Yes
Widget Yes No

To me it makes more sense to either 1) have widgets and in-line code behave the same, or 2) completely isolate the widget context from the parent and only propagate contexts between in-line children and parents. However, option 2 completely defeats the purpose of the variable system in pages, so that really just leaves option 1.

On the illustration there you can see how it's not desirable for the two personal widgets at the bottom to manipulate the same "inputText" variable.

I agree with the logic of that particular case, but I would argue that is a pretty rare case. Furthermore, that case can be fixed with appropriate widget design. If a widget variable needs to be isolated, then it is easy enough to add a variable prefix property to the widget so that all variables that need to be isolated would just be referenced as vars[(props.vPrefix || '') + 'myIndependentVariable'].

Now I don't know how many users out there currently take advantage of the widget variable isolation, so changing this default behavior might be too big a breaking change.

What about adding a configuration property that modifies this behavior?

    - component: widget:my_example_widget
      config:
        title: Standard Isolated Widget
    - component: widget:demo_simple_variable
      config:
        title: Open Context Widget
        shareVarsContext: true <-- this widget would propagate variables back up to the Parent

@SimonB27P
Copy link
Author

SimonB27P commented Mar 2, 2024

As Justin says, isolation can be done by other means. By separating the variable context from the parent widget, there is no means for a custom sub widget to manipulate a variable in the context of the parent.

When you design a personal widget in general I don't think you want it to depend on its parent's context.

I think actually you do, much like you would expect a variableAction in a button to affect the variable in the button's parent widget. Take my use-case. I have a list. Each list item contains a toggle. Those toggles need to manipulate a variable that is outside the list (indeed the status of the list items themselves depends on the variable). If the list items use oh-buttons or oh-toggles then they can see the parent widget variable. However, if I use a custom widget for the toggle, which I would like to do because my list items are complex, then, I believe you're right, the variable is passed but only by value. The custom widget cannot manipulate the variable. That makes no sense. The only workaround is to repeat the custom widget code in the parent widget. if you have several identical custom sub-widgets, that makes for a lot of unnecessary code.

What would massively increase the useability and usefulness of variables would be something along the lines of:

In the parent

	- component: widget:customWidget
	  config:
	    variableName1: =vars.parentVariable1

... or simply make the variable visible in the custom widget, and then, in the custom widget...

uid: customWidget
tags: []
props:
  ...
vars:
  - description: Description of the variable
    name: variableName1
    byValue: false
  - description: Description of the variable
    name: variableName2
    default: "Default value"

The goal would be to define variables in the widget, not just on first use (that also takes away the need to trap empty variables everywhere). It would be possible to define a default starting value. If we need a local variable in the custom widget that is either only used in the custom widget or needs to be passed by value and not changed in the parent, we could do that explicitly.

This would have much clearer logic for the user.

@ghys
Copy link
Member

ghys commented Mar 3, 2024

Thanks for the valuable feedback.

Scope isolation is not a new problem (it was a major source of headaches in the old AngularJS, see https://www.3pillarglobal.com/insights/angularjs-understanding-directive-scope/).

Note that I wasn't completely sure that you couldn't pass data by reference because #1556 introduced objects in variables and I was wondering if you could take advantage of that, turns out you have sometimes inner properties of object variables bubbling up to the parent context (if you share the same variable but use different variableKey), I ended up testing but it's quite random and really quite buggy.

Expand for some test widgets

You can test them yourself if you wish.

Parent widget

uid: toggleJsonVariables
tags: []
props:
  parameters: []
  parameterGroups: []
timestamp: Mar 3, 2024, 3:07:31 PM
component: f7-card
config:
  title: Test variables
slots:
  default:
    - component: f7-card-content
      slots:
        default:
          - component: widget:toggleJsonVariable
            config:
              variableKey: test1
          - component: widget:toggleJsonVariable
            config:
              variableKey: test2
          - component: oh-input
            config:
              outline: true
              type: text
              variable: test
              variableKey: outerInput
          - component: oh-toggle
            config:
              variable: test
              variableKey: outerToggle
              
    - component: f7-card-footer
      config:
        variableKey: test1
      slots:
        default:
          - component: Label
            config:
              text: =JSON.stringify(vars)

Child widget:

uid: toggleJsonVariable
tags: []
props:
  parameters:
    - description: Variable key to change
      label: Variable Key
      name: variableKey
      required: false
      type: TEXT
  parameterGroups: []
timestamp: Mar 3, 2024, 3:01:08 PM
component: f7-card
config:
  title: =props.variableKey
slots:
  default:
    - component: f7-card-content
      slots:
        default:
          - component: oh-input
            config:
              name: ="input-" + props.variableKey
              type: text
              outline: true
              variableKey: ="input-" + props.variableKey
              variable: test
          - component: oh-toggle
            config:
              variableKey: ="toggle-" + props.variableKey
              variable: test
    - component: f7-card-footer
      slots:
        default:
          - component: Label
            config:
              text: =JSON.stringify(vars)

image

I agree that there might be more use cases where you'd prefer not have "scope", or a single variable context for the entire page, so you should have that option, or it should become the default if there are enough guarantees that it would not break too many things.
It could become problematic if, for example, someone had published a widget in the marketplace that uses variables thinking they're "local" to their widget, and someone else reuses that widget multiple times on a page.

Your solution proposals are interesting, however adding a vars section to the widgets definition is something that has to be added in RootUIComponent in core, and variables are purely a Pages concepts so I it wouldn't make sense to all types of components. Maybe there's something to be done with the context property of parameters (props). It could be set to a special value e.g. variable and then the prop would be considered as a shared variable somehow, maybe.

It's also been bugging me that you can define variables but only on the (root) page level, with the defineVars config parameter (added in #639). You could take advantage of that if you were offered the ability to do it at the widget level too - or even at the component level, to (re-)define variables in isolation for this part of the component tree (but you would still be able to also access variables defined above in the tree), so that would be another solution. You wouldn't need @JustinGeorgi's shareVarsContext either with this approach, you'd simply define the variables that you use at any level (allowing to set their initial value too).

@SimonB27P
Copy link
Author

Thanks for your thoughts.

I suspect this is quite niche but I think it could be quite powerful and therefore more widely used if implemented. In my case, I am trying to set up a selction screen, which enables the user to pick items to change. But I want the user to be able to either cancel the process or to be able to rethink and say to him/herself, oops, no not item B, I meant item C. If the components are linked directly to items, it's too late; you've already turned off the lights or turned on the sprinklers(!). So I'm doing it all with variables. When you're happy with the selection, you hit OK and an array of items and values is returned for further processing, or you hit Cancel and nothing changes.

This all works until you need to use a custom widget to manipulate your variable. I have a three-way toggle widget that replaces the standard oh-toggle but displays an UNDEF mid-point if the group it represents is not either all ON or all OFF. It's a simple widget designed to be a drop-in replacement for the oh-toggle. An oh-toggle can manipulate the variable in the parent widget using variable parameter. My custom togggle cannot. The only workaround at the moment is to reproduce the custom widget code everywhere it is needed in the parent, which is clunky to say the least.

Any developments on this would be much appreciated.

@JustinGeorgi
Copy link
Contributor

You could take advantage of that if you were offered the ability to do it at the widget level too - or even at the component level, to (re-)define variables in isolation for this part of the component tree (but you would still be able to also access variables defined above in the tree), so that would be another solution. You wouldn't need @JustinGeorgi's shareVarsContext either with this approach, you'd simply define the variables that you use at any level (allowing to set their initial value too).

Something like this would cover a large number of user requests over the years (particularly being able to give a variable an initial value).

Do you think it would make sense to do something like (or combine it with) my proposal #2148? I could see adding sections to that such as globalVariable and localVariable to cover different variable scope cases.

@ghys
Copy link
Member

ghys commented Mar 7, 2024

Do you think it would make sense to do something like (or combine it with) my proposal #2148?

Yes this is also interesting (sorry I missed it) and it also could be combined with the proposal above!

@JustinGeorgi
Copy link
Contributor

JustinGeorgi commented Mar 8, 2024

I'll take a crack at it then. I started something regarding the other proposal while back but got side-tracked myself. I'll dust it off and see I can I work out how to include variables as well.

I"m traveling shortly, so it may have to wait until I get back but I'll see what I can do.

@SimonB27P
Copy link
Author

If you're looking at this Justin, another issue just occurred to me that it would be good to make sure is dealt with at the same time (it might be anyway). If you're using variables in a widget that then triggers a popup, the popup is not able to use or manipulate variables in the original widget. Clearly it would be very helpful to be able to do that.

I have in mind (and would have liked to be able to use this feature to do this) a control widget that has a popup to select which items to apply a property to and then perhaps another popup to select what time or date to apply. Only when all that information has been collected would the changes be made, enabling the user to cancel the process if desired.

Variables would be the way to do this. As it is I am going to have to use a workaround sending data to items as a conduit to rules which then hold values until all the other similar items have returned data.

@JustinGeorgi
Copy link
Contributor

If you're using variables in a widget that then triggers a popup, the popup is not able to use or manipulate variables in the original widget.

I'm not in front of my system at the moment, but as I recall this depends on how you define the popup (and in part, this has to do with how f7 does it's popups). If you build the popup inline with the rest of this widgets, I believe it has access to the variables of that widget.

florian-h05 pushed a commit that referenced this issue Jun 1, 2024
Closes #2437, Closes #2148

This PR adds a new component, the oh-context. Similar to the repeater,
this component is not rendered, but injects information into the widget
at it's tree location.

The component allows to inject these three things into the widget:
* functions - using the arrow function syntax, named functions can be
declared and reused in all subsequent expressions
* constants - constants can be defined as either single values, arrays,
or objects
* variables - variables can be defined with default values. These
variables are local in scope to the oh-context and it's descendants and
take precedence over other variables of the same name from higher
contexts.
  * Variables are not divided into global vs local explicitly. But a
oh-context used as the root component of a widget will have its
variables in the context of all the other components on that widget and
thus they essentially have a global context within that widget.
  * In contrast to the basic widget variables, oh-context variables do
have bi-directional passage between a main widget and a sub widget.

---------

Also-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Justin Georgi <justin.georgi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants