node-red / node-red-ui-nodes

Additional nodes for Node-RED Dashboard
Apache License 2.0
122 stars 83 forks source link

PR for calling functions on tabulator (commands) #24

Closed Christian-Me closed 4 years ago

Christian-Me commented 4 years ago

As announced before I like to propose a "small" non-breaking change for the UI-Table widget to make it possible to alter the ui-table data without re-sending the hole table array.

The example is "too" complex and not finished. I will provide a simpler one with inject nodes to show the possibilities when we agree on the api.

image

I was able to send more than 50.000 lines to the widget until the browser spit out a out of memory error. I used a syslog node as a data feed and the flow takes care on refresh, new connects and to limit the amount of data held by ui-table.

Main question is if the API for calling tabulator functions is ok.

Proposed changes

Instead of sending an array to ui-table to replace the complete table data ui-table also accepts an object as payload to send commands. The object must have the following properties

example

{"payload":{
    "command":"addData",
    "arguments":[
        {
            "facility":"daemon",
            "facilityCode":3,
            "severity":"info",
            "severityCode":6,
            "tag":"systemd[1]",
            "timestamp":"2020-01-02T19:17:39.793Z",
            "hostname":"localhost",
            "address":"127.0.0.1",
            "family":"IPv4",
            "port":38514,
            "size":80,
            "msg":"some demo data",
            "id":2351
        },
        true
    ],
    "returnPromise":true
    }
}

Checklist

Christian-Me commented 4 years ago

Hello, @dceejay , @kazuhitoyokoi sorry I always wonder when it is appropriate ask if there is progress on a PR. I know we all have a lot of things on our to do lists.

I`m just about to finish a project using dynamic updates of ui-table quite extensively. Many IOT and home automation projects have to deal with many remote devices .... image

The flow is configurable and uses partial updates using the "updateOrAddData" function of tabulator. Currently it can handle mqtt, http, homematic and other data sources. And by the way it makes a lot of use of the ui_controll feature added by the last PR.

image

The central ui-table handler is a subflow and can therefore be used (or modified) for other use cases .

dceejay commented 4 years ago

nice - still waiting for response to comments.

kazuhitoyokoi commented 4 years ago

I apologize for my late response. This weekend, I will also check this pull request too.

Christian-Me commented 4 years ago

Hi, I was just preparing a simple example flow to play/demonstrate around different commands when I ran into an issue with the table height. When ui-table is used the standard way (passing an array) the height is determined by

height: tabledata.length * y + 26

Which is a little bit "strange" because when a user passes a long table the height gets very "long" too and perhaps only usefull in auto mode.

But using commands there is no tabledata existing so no amount of rows so no height. In all my tables I use ui_control with customHeight property defining the height so I never touched that issue. But by using a simple setup without ui_control no data will be shown when I add data because the height is 26 only showing the header. Defining the height to a ridiculously high value the table (set to a size other than auto) fill the widget space as expected and the scroll bars appear as expected then the space is exceeded

height: (tabledata.length > 0 )? tabledata.length * y + 26 : 2000

But I think that is not the way it should be done. I would rather pass the correct height: height=config.height*48+config.height*5 but I could now find out how to get the 1x1 widget size and widget spacing values as defined in the dashboard configuration.

Any ideas or suggestion? Or should I leave it with 5000 or so for empty tables for now?

dceejay commented 4 years ago

there is a function ui.getSizes() that should return the sizes

dceejay commented 4 years ago

@Christian-Me Did you want me to merge this as-is ? or wait until you've looked at the getSizes() ?

Christian-Me commented 4 years ago

@dceejay Thank you, with getSizes() it is working now as expected, thank you for the hint. Please wait a little I`m preparing a simple example flow / test bench as promised. Also I write/finalize the readme.md and the description side tab. image

dceejay commented 4 years ago

no rush at my end :-)

Christian-Me commented 4 years ago

Sometimes it takes longer than expected. image No hurry to merge this perhaps you like to fix the other issue first

bc547 commented 4 years ago

No hurry to merge this perhaps you like to fix the other issue first

I know at least 5 people who are really looking forward to this extra functionality and are following this thread closely :-) A big thanks already for implementing this!

(what other issue btw? the error handling mentioned above?)

Christian-Me commented 4 years ago

I mean this one https://github.com/node-red/node-red-ui-nodes/issues/29 and another one https://github.com/node-red/node-red-ui-nodes/issues/30

Which is hopefully both not related to this and my previous ui_controll PR but you never know. I think the error handling is ok for now because these are tabulator errors in example when you try to delete a row with an id/index not existing.

As my tables are all dynamic and when you use commands the flow has to take care of a correct refresh using the ui-control node I don't know how long the redraw problem exists. But for the ui_table example I did a lot of tests and didn`t had this problem.