bartbutenaers / node-red-contrib-ui-svg

A Node-RED widget node to show interactive SVG (vector graphics) in the dashboard
Apache License 2.0
94 stars 27 forks source link

Scripts: panzoom and hammer slowing down dashboard load time #86

Closed Ashfaak closed 3 years ago

Ashfaak commented 3 years ago

Hi Bart

I refer to the following lines (181-182) in svg_graphics.js:

<script src= "ui_svg_graphics/lib/panzoom"></script>
<script src= "ui_svg_graphics/lib/hammer"></script>

I get several "404 Not Found" on every dashboard page load when trying to load these scripts which do not exist. This translates to ~2 seconds unnecessary latency in my case.

I'm using neither panzoom nor hammer. Would it be possible to remove these lines in the case that these options are not used?

node-red: 1.2.6 node-red-contrib-ui-svg: 2.1.1 node-red-dashboard: 2.26.0

image

bartbutenaers commented 3 years ago

Hi @Ashfaak, Will have a look at that. Has been asked already in the past.

But what bothers me is that it cannot find the files. Do you see a "Javascript file ... does not exist" message in your browser console log? Because it might be a problem for other users that need panzoom...

Thanks and happy newyear! Bart

Ashfaak commented 3 years ago

Hi @bartbutenaers, Happy New Year!

Did some digging.

If I call this: https://example.com/ui/ui_svg_graphics/lib/panzoom or https://example.com/ui/ui_svg_graphics/lib/hammer I get the correct responses.

The 404s above were calling https://example.com/ui_svg_graphics/lib/panzoom and https://example.com/ui_svg_graphics/lib/hammer.

In settings.js the UI path is set like this: UI: { path: "" },

Line 1792 of svg_graphics.js: uiPath = ((RED.settings.ui || {}).path) || 'ui'; -> uiPath = "ui". But in my case this should be uiPath = "".

Console.log within the js portion does not output anything to the Chrome or Firefox consoles. I could only get it to work in the html portion. Not sure if there's some other way to access these logs.

For now I've removed both <script> lines and it runs perfectly since I'm not using the function.

Thanks, I hope this helps. Ashfaak

Edit: just tried setting UI: { path: "/" }, and now hammer and panzoom are found successfully. But, it is still slow. I will remove both lines.

bartbutenaers commented 3 years ago

Hello @Ashfaak, Sorry for the delay. Was busy with my new joystick node ...

The version on Github now only loads the both libraries when panzoom is enabled. So hopefully that solves your problem. Will publish that on NPM soon, but need to finalize the current release of the SVG node first ...

I really appreciate your detail of feedback about the ui path!!!!! Such info is a big help for me trying to make the node more stable for other users. Thanks for your time to analyze the issue, even though you don't need it yourself...

bartbutenaers commented 3 years ago

@Ashfaak,

Would like to have the uiPath problem solved, because then I can release the 2.2.0 version.

But don't understand it fully. When I calculate both situations from your explanation:

image

I would expect the first option (with path="") to be working, because that url contains .../ui/... And I would expect the second option (with path="/") to be failing, because that url doesn't contain .../ui/...

But from your explanation I understand that it is the other way around... Any advise is welcome!!

Ashfaak commented 3 years ago

Hi @bartbutenaers

Glad I could help!

I think some background might be useful:

This dashboard is built for other users so I wanted the dashboard to be at example.com, rather than example.com/ui. The Node-RED config page is at example.com/admin. The users never see the config page and example.com/ui is not used at all.

<script src= "ui_svg_graphics/lib/panzoom"></script> says the the script is at dashboardURL/ui_svg_graphics/lib/panzoom, but because of line 1792 the script was at dashboardURL/ui/ui_svg_graphics/lib/panzoom.

If my understanding is correct, line 1792 checks if 'RED.settings.ui' is blank and gives uiPath the default value of 'ui 'if it is. Perhaps it should check if 'RED.settings.ui' is undefined instead?

bartbutenaers commented 3 years ago

Evening @Ashfaak, You are absolutely correct. I was totally confused. I have added a fix Thanks for your feedback!!! Bart

bartbutenaers commented 3 years ago

P.S. Version 2.2.0 has been published on NPM.