FlowFuse / node-red-dashboard

https://dashboard.flowfuse.com
Apache License 2.0
159 stars 35 forks source link

UI-Template: Should give error if invalid `page`/ `ui` is selected #1039

Open arturv2000 opened 4 days ago

arturv2000 commented 4 days ago

Current Behavior

Currently the ui-template only gives error if no group selected (in case Type = Widget - Group-Scoped) image

When in (Group-Scoped) there will be a red triangle associated to the node, and an error appear when attempting to a Deploy.

The same error should appear in the other configurations in case no page or ui is selected

image

Currently no error is indicated, leading that the user may do a deploy and start wondering why is not working....

Figured out where one of the error is:

https://github.com/FlowFuse/node-red-dashboard/blob/b15c2193116df668407498749f87689a511b61ff/nodes/widgets/ui_template.html#L79

The variable this.templateScope is not updated in real time with the selection, it always has the initial value local when is a new template.

Also there is no validation for the css types.

It doesn't show the error while the edit dialog is open, but it shows onde closed with the red triangle on the node, and it will show the error when reopening the edit dialog.

The behaviour is different from Node-Red 4.0.1 and Node-Red 3.1.7

In 3.1.7 by default when selecting a new Type the ui/page automatically selects the first entry, witch normally is a valid option. With 4.0.1 the default option is none

Expected Behavior

No response

Steps To Reproduce

No response

Environment

Have you provided an initial effort estimate for this issue?

I am not a FlowFuse team member

arturv2000 commented 4 days ago

Just for fun, attempted to find a fix / solution for this...

group: {
    type: 'ui-group',
    validate: function () {
        const tempScope = ($('#node-input-templateScope').val() !== undefined) ? $('#node-input-templateScope').val() : this.templateScope
        const groupVal = ($('#node-input-group').val() !== undefined) ? $('#node-input-group').val() : this.group

        if (tempScope !== 'local') {
            return true
        }

        if (tempScope === 'local') {
            if (!!groupVal && groupVal !== '_ADD_') {
                return true
            }
        }
        return false
    }
}, // for when template is scoped to 'local' (default)
page: {
    type: 'ui-page',
    validate: function () {
        const tempScope = ($('#node-input-templateScope').val() !== undefined) ? $('#node-input-templateScope').val() : this.templateScope
        const pageVal = ($('#node-input-page').val() !== undefined) ? $('#node-input-page').val() : this.page

        if (tempScope !== 'widget:page' && tempScope !== 'page:style') {
            return true
        }
        if (tempScope === 'widget:page' || tempScope === 'page:style') {
            if (!!pageVal && pageVal !== '_ADD_') {
                return true
            }
        }
        return false
    }
}, // for when template is scoped to 'page'
ui: {
    type: 'ui-base',
    validate: function () {
        const tempScope = ($('#node-input-templateScope').val() !== undefined) ? $('#node-input-templateScope').val() : this.templateScope
        const uiVal = ($('#node-input-ui').val() !== undefined) ? $('#node-input-ui').val() : this.ui

        if (tempScope !== 'widget:ui' && tempScope !== 'site:style') {
            return true
        }
        if (tempScope === 'widget:ui' || tempScope === 'site:style') {
            if (!!uiVal && uiVal !== '_ADD_') {
                return true
            }
        }
        return false
    }
},

If this is a viable solution, I can open a PR for this...