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

Error in capture demo #95

Closed
jywarren opened this issue Jun 11, 2019 · 15 comments
Closed

Error in capture demo #95

jywarren opened this issue Jun 11, 2019 · 15 comments
Assignees

Comments

@jywarren
Copy link
Member

Seeing this error on the capture demo at: https://publiclab.github.io/spectral-workbench.js/examples/capture/

Uncaught Error: Invalid dimensions for plot, width = 0, height = 200
    at getCanvasDimensions (jquery.flot.js:803)
    at setupCanvases (jquery.flot.js:853)
    at new Plot (jquery.flot.js:224)
    at Function.$.plot (jquery.flot.js:2690)
    at getRow (capture.js:341)

cc @sidntrivedi012 @starkblaze01

@sidntrivedi012
Copy link
Member

The jquery.flot.js file has this snippet at 135 line :


		if (width <= 0 || height <= 0) {
			throw new Error("Invalid dimensions for plot, width = " + width + ", height = " + height);
		}

This is the error that is being shown on the console.
The line that is causing error in the capture.js file :

$W.plot = $.plot($("#graph"),$W.data,flotoptions); 

There is something wrong with the plotting of the graph. Looking into it.

@starkblaze01
Copy link
Member

The jquery.flot.js file has this snippet at 135 line :


		if (width <= 0 || height <= 0) {
			throw new Error("Invalid dimensions for plot, width = " + width + ", height = " + height);
		}

This is the error that is being shown on the console.
The line that is causing error in the capture.js file :

$W.plot = $.plot($("#graph"),$W.data,flotoptions); 

There is something wrong with the plotting of the graph. Looking into it.

Cool!
Can you verify why width = 0 is passed in the data

@jywarren
Copy link
Member Author

So the line where the error is originating (from the last line of the pasted error above) is:

$W.plot = $.plot($("#graph"),$W.data,flotoptions);

Tracking down these lines and getting the permalink and inline code display is really helpful for leaving good notes about your progress, so I recommend it!

@jywarren
Copy link
Member Author

Perhaps the thing to do is to try logging out flotoptions above this line. Then to as @starkblaze01 suggests, trace back to where width is being generated.

@sidntrivedi012
Copy link
Member

So the line where the error is originating (from the last line of the pasted error above) is:

$W.plot = $.plot($("#graph"),$W.data,flotoptions);

Tracking down these lines and getting the permalink and inline code display is really helpful for leaving good notes about your progress, so I recommend it!

Sure, will keep in mind from the next time.Thanks 🙂

@sidntrivedi012
Copy link
Member

Whenever I am changing the width here to like a px value like 100 px, it is working fine. But in case of width:100%, it is giving errors.

<div id="graph" style="width:100%;height:200px;margin:10px 0;"></div>

@jywarren
Copy link
Member Author

jywarren commented Jun 19, 2019 via email

@sidntrivedi012
Copy link
Member

Sure @jywarren. Also, adding to this there's also one more error popping up :

Access to XMLHttpRequest at 'file:///capture/recent_calibrations.json?calibration_id=undefined' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https.

@jywarren
Copy link
Member Author

OK, that one is where it's trying to fetch recent calibrations from the server. So we'll need to refactor it to accept a constructor parameter on this line:

$W.initialize({
calibrated: false,
interface:"capture",
mode:"combined",
flipped: false,
width: 640
})

And then save it in the initialize method:

initialize: function(args) {
this.mobile = args['mobile'] || false
this.flipped = args['flipped'] == true || false
this.interface = args['interface'] || false

And then on the line where it's requested, maybe if there is no provided URL, we don't actually send this AJAX request at all:

getRecentCalibrations: function(selector) {
$.ajax({
url: "/capture/recent_calibrations.json?calibration_id=" + $W.calibration_id,
type: "GET",
success: function(data) {

Actually the method is getRecentCalibrations() and it's run here:

setInterval(function() {
$W.getRecentCalibrations('.select-calibration');
}, 10000); // every 10 seconds

So we could say if ($W.recentCalibrationsUrl !== false)

and in initialize, we could have it default to false:

    this.recentCalibrationsUrl = args['recentCalibrationsUrl'] || false

@cryptoclidus
Copy link

cryptoclidus commented Jun 21, 2019

I think I got why it's defaulting to 0 when you set width to 100%:

On the html, there are two divs with class="tab pane", with id="settings" and id="capture". the tab pane "settings" is by default active, so capture is inactive (screenshot). So by default, "display:none" for the entire capture tab pane.

This means that any div with no specified width, or min width, (e.g. the elements with width:100%) will default to 0 when display:none. Since the #graph is inside the "capture" tab pane, which is inactive, it's width is default 0.

One possible fix can be to set a min-width of 1 px for the #graph div.

bug_95_b

@cryptoclidus
Copy link

That's also why when you click "Begin Capturing" and the display switches to the "capture" tab pane, the error goes away

@jywarren
Copy link
Member Author

jywarren commented Jun 21, 2019 via email

@cryptoclidus
Copy link

Another thought on this: while setting the min-width:1 px is a quick and easy fix, this error is evidence that the program is repeatedly trying to draw the graph, when the graph isn't even being displayed. While it might not really have a substantial effect on performance, is it really necessary to draw the graph when the user is not on the "capture" tab pane? So a more complex fix might be to find all the graph-related and waterfall-related code and put them inside an "if" statement or an event listener that only runs when the "capture" tab pane is active. What do you think?

@jywarren
Copy link
Member Author

@sidntrivedi012 can you please check in on this? Thanks @cryptoclidus !!!

@jywarren
Copy link
Member Author

Solved in #102!!!

@sidntrivedi012 great work.

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

No branches or pull requests

4 participants