FlowFuse / node-red-dashboard

https://dashboard.flowfuse.com
Apache License 2.0
205 stars 49 forks source link

ui-dropdown - Dropdown shows "No options available" when options are provided dynamically #1011

Closed adamf-datapath closed 4 months ago

adamf-datapath commented 4 months ago

Current Behavior

When providing options dynamically to ui-dropdown by setting msg.options, the rendered dropdown states there are "No options available", even though there are.

image

Expected Behavior

Dropdown does not warn about no options available.

Steps To Reproduce

  1. Pass a list of options (via msg.options) to ui-dropdown, in response to ui-event.
  2. View the dashboard. The options will be present in the dropdown, but the error will be shown.

Reproduction:

[
    {
        "id": "35a00fcc21bff589",
        "type": "tab",
        "label": "Flow 1",
        "disabled": false,
        "info": "",
        "env": []
    },
    {
        "id": "2aa91d68bd7d2cf7",
        "type": "ui-dropdown",
        "z": "35a00fcc21bff589",
        "group": "e5ee6bb9bf33fc5f",
        "name": "",
        "label": "Select Option:",
        "tooltip": "",
        "order": 1,
        "width": 0,
        "height": 0,
        "passthru": false,
        "multiple": false,
        "options": [],
        "payload": "",
        "topic": "topic",
        "topicType": "msg",
        "className": "",
        "x": 960,
        "y": 200,
        "wires": [
            [
                "e0d608487ccd1149"
            ]
        ]
    },
    {
        "id": "e0d608487ccd1149",
        "type": "debug",
        "z": "35a00fcc21bff589",
        "name": "debug 3",
        "active": true,
        "tosidebar": true,
        "console": false,
        "tostatus": false,
        "complete": "true",
        "targetType": "full",
        "statusVal": "",
        "statusType": "auto",
        "x": 960,
        "y": 260,
        "wires": []
    },
    {
        "id": "9cd6c91afcdbfb2f",
        "type": "change",
        "z": "35a00fcc21bff589",
        "name": "",
        "rules": [
            {
                "t": "set",
                "p": "options",
                "pt": "msg",
                "to": "[{\"value\":1,\"label\":\"One\"},{\"value\":2,\"label\":\"Two\"}]",
                "tot": "json"
            },
            {
                "t": "set",
                "p": "topic",
                "pt": "msg",
                "to": "test-topic",
                "tot": "str"
            }
        ],
        "action": "",
        "property": "",
        "from": "",
        "to": "",
        "reg": false,
        "x": 720,
        "y": 200,
        "wires": [
            [
                "2aa91d68bd7d2cf7",
                "e0d608487ccd1149"
            ]
        ]
    },
    {
        "id": "5456420d981161db",
        "type": "ui-event",
        "z": "35a00fcc21bff589",
        "ui": "e578ecf151797c4b",
        "name": "",
        "x": 550,
        "y": 200,
        "wires": [
            [
                "9cd6c91afcdbfb2f"
            ]
        ]
    },
    {
        "id": "489a7c8d00a851bb",
        "type": "inject",
        "z": "35a00fcc21bff589",
        "name": "",
        "props": [
            {
                "p": "payload"
            },
            {
                "p": "topic",
                "vt": "str"
            }
        ],
        "repeat": "",
        "crontab": "",
        "once": true,
        "onceDelay": 0.1,
        "topic": "",
        "payload": "",
        "payloadType": "date",
        "x": 570,
        "y": 120,
        "wires": [
            [
                "2aa91d68bd7d2cf7"
            ]
        ]
    },
    {
        "id": "e5ee6bb9bf33fc5f",
        "type": "ui-group",
        "name": "Test",
        "page": "6722c437d7090ba4",
        "width": "6",
        "height": "1",
        "order": 1,
        "showTitle": true,
        "className": "",
        "visible": "true",
        "disabled": "false"
    },
    {
        "id": "e578ecf151797c4b",
        "type": "ui-base",
        "name": "My Dashboard",
        "path": "/dashboard",
        "includeClientData": true,
        "acceptsClientConfig": [
            "ui-notification",
            "ui-control",
            "ui-button",
            "ui-dropdown"
        ],
        "showPathInSidebar": false,
        "navigationStyle": "fixed",
        "titleBarStyle": "default"
    },
    {
        "id": "6722c437d7090ba4",
        "type": "ui-page",
        "name": "Test",
        "ui": "e578ecf151797c4b",
        "path": "/test",
        "icon": "home",
        "layout": "grid",
        "theme": "311d097b7376955f",
        "order": 1,
        "className": "",
        "visible": "true",
        "disabled": "false"
    },
    {
        "id": "311d097b7376955f",
        "type": "ui-theme",
        "name": "Default Theme",
        "colors": {
            "surface": "#ffffff",
            "primary": "#0094ce",
            "bgPage": "#eeeeee",
            "groupBg": "#ffffff",
            "groupOutline": "#cccccc"
        },
        "sizes": {
            "pagePadding": "12px",
            "groupGap": "12px",
            "groupBorderRadius": "4px",
            "widgetGap": "12px"
        }
    }
]

Environment

Have you provided an initial effort estimate for this issue?

I am not a FlowFuse team member

arturv2000 commented 4 months ago

Just tested your example flow, can confirm that it happens.

One work-around is to define an empty option on the node config...

image

It seems to remove that error.

Strange never had found that error, well, by default when we add a new Dropdown node to the dashboard the node by default has one empty option, unless we remove it the error would not trigger.

Another semi-related issue is that when we send a msg with options as being an empty array this error does not show.

bartbutenaers commented 4 months ago

Hmm not sure how to fix this, because I have never used the ui-control node myself.
Seems to be 2 different problems.

Problem 1: incorrect error label

The 2 options are extracted correctly from the input message, and returned by the computed options property:

image

Which should trigger the label to be hidden:

image

Although I 'think' that the first ? is a typo? I did remove it, but the label is still being displayed.

Problem 2: no value selected

This is what happens:

  1. The ui-control node injects this message:

    image

  2. Which arrives in the recently added dynamic property mechanism:

    image

  3. The options are correctly extracted from the message:

    image

  4. But then the node tries to determine which of these options need to be selected, by looking at the payload:

    image

    Since the msg.payload value is not available in the list of options, nothing will be selected.

So I assume the input message payload should be fixed by the user's flow? Although imho it would be user friendly to show a node.error that the msg.payload contains no valid option...

arturv2000 commented 4 months ago

Regarding the first problem, i would say the error is in this line:

https://github.com/FlowFuse/node-red-dashboard/blob/240a03ca1b55702ef444039d08a7e38623ec3c58/ui/src/widgets/ui-dropdown/UIDropdown.vue#L15

I think it should be:

:error-messages="options?.length ? '' : 'No options available'"

since after we send a message with the options field, the item list is in variable items that is fetched by the compute propriety options.

Making this change (tested on vuetify playground), it seems that the warning red color will appear in case we send a message with options as an empty array, and in case the node config windows doesn't have any option. Will be refreshed after we send a message with options at startup.

I don't think the 2 problem, is a really "issue". Having no value selected, in the example flow that indeed happens because the payload is not valid considering the options, I would argue that the user should send a valid payload or not send any payload.

In my opinion it is valid that I may only want to update the options (Item List) without assigning any value. And, it would be useful to have some way to purposely clear the comboBox and return to the original state (with no item selected).

bartbutenaers commented 4 months ago

I think it should be

Yes you are correct that is the one. Build and tested it, and then it works. Thanks!! I have created a pull request.

bartbutenaers commented 4 months ago

In my opinion it is valid that I may only want to update the options (Item List) without assigning any value

Yes true. But if you inject a payload with an invalid content, imho it would be userfriendly to show a warning in the sidebar.

arturv2000 commented 4 months ago

In my opinion it is valid that I may only want to update the options (Item List) without assigning any value

Yes true. But if you inject a payload with an invalid content, imho it would be userfriendly to show a warning in the sidebar.

Yes agree, the same way other nodes send error/warn messages to the debug tab.

Although the action of sending only the options should maybe also clear the Dropdown value, specially if the selected value is not present in the new options to avoid inconsistency.. Or maybe more simple to have a way to clear the selection, like an empty string on payload. Disregard this commentary, just notice it that is necessary to send an empty array.