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

luci-app-olsr-viz: migrate to JavaScript-based implementation #6480

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

Ayushmanwebdeveloper
Copy link
Contributor

image

Hi! I've completed the migration of luci-app-olsr-viz to JavaScript. All the features are functioning just like they did in the old Lua app.

And in the old app, there was an issue of HTTP 413 Content Too Large after running it twice, which resulted in a blank page because the app was saving 130 Cookies at a time as it was saving all the nodes in cookies. I tested it on many routers and was able to reproduce the issue every time.

Screenshot 2023-07-25 204605

So, I removed the saving of the nodes in the cookies, as it was making the app unusable.

Now everything is working normally, and I've fixed the error.

@Ayushmanwebdeveloper Ayushmanwebdeveloper force-pushed the luci-app-olsr-viz-mig branch 3 times, most recently from 86c2144 to d0ca899 Compare July 26, 2023 12:32
var debugSpan = E('span', { 'id': 'debug', 'style': 'visibility:hidden;' });

var vizDiv = E('div', { 'id': 'RSIFrame', 'name': 'RSIFrame', 'style': 'border:0px; width:0px; height:0px; visibility:hidden;' });
var vizScript = E('script', { 'type': 'text/javascript' }, 'viz_setup("RSIFrame", "main","nodes", "edges");viz_update();');
Copy link
Contributor

@jow- jow- Jul 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to modify viz_setup() to take the target div directly instead of a div name. Then simply replace this delayed script instantiation with a synchronous call to viz_setup(vizDiv, "main", "nodes", "edges"); viz_update();

and the vizDiv instantiation with var vizDiv = E('div', { 'id': 'RSIFrame', 'name': 'RSIFrame', 'style': 'border:0px; width:0px; height:0px; visibility:hidden;' }, viz_res);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the issues & also modified script file for directly using divs & inputs instead of getting them by id. and I implemented callingviz_setup(vizDiv, "main", "nodes", "edges"); viz_update(); synchronously.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I will test it for some time more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it & fixed all the issues. Thanks for the review.

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix tag

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix var name

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js use load for script & fix bugs

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix script

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix bugs

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix view

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js fix script

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js fix view

luci-app-olsr-viz: migrate to js

luci-app-olsr-viz: migrate to js fix script

Signed-off-by: Ayushman Tripathi <[email protected]>

luci-app-olsr-viz: migrate to js
@systemcrash
Copy link
Contributor

my $0.02 - I like this.

@andibraeu
Copy link
Contributor

@jow- this migrated app looks good to me

@jow- jow- merged commit 08fbcee into openwrt:master Sep 26, 2023
2 checks passed
@jow-
Copy link
Contributor

jow- commented Sep 26, 2023

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants