Skip to content

Etienne devtools improvements #418

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

Merged
merged 5 commits into from
Apr 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions devtools/test_dashboard/devtools.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
'use strict';

/* global Plotly:false */
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't sure what our stance was on file-specific eslint settings. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you make npm run lint happy, then 👍


var Fuse = require('fuse.js');
var mocks = require('../../build/test_dashboard_mocks.json');

// put d3 in window scope
var d3 = window.d3 = Plotly.d3;
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 use this a lot to debug. Typing Plotly.d3 every time is annoying.


// Our gracious testing object
var Tabs = {
Expand Down Expand Up @@ -35,8 +41,8 @@ var Tabs = {
plotMock: function(mockName, id) {
var mockURL = '/test/image/mocks/' + mockName + '.json';

window.Plotly.d3.json(mockURL, function(err, fig) {
window.Plotly.plot(Tabs.fresh(id), fig.data, fig.layout);
d3.json(mockURL, function(err, fig) {
Plotly.plot(Tabs.fresh(id), fig.data, fig.layout);

console.warn('Plotting:', mockURL);
});
Expand All @@ -47,12 +53,13 @@ var Tabs = {
var gd = Tabs.getGraph(id);

if(!gd._fullLayout || !gd._fullData) {
console.error('no graph with id ' + id + ' found.');
return;
}

var image = new Image();

window.Plotly.Snapshot.toImage(gd, { format: 'png' }).on('success', function(img) {
Plotly.Snapshot.toImage(gd, { format: 'png' }).on('success', function(img) {
image.src = img;

var imageDiv = document.getElementById('snapshot');
Expand All @@ -72,7 +79,7 @@ var Tabs = {
}

for(var i = 0; i < plots.length; i++) {
window.Plotly.purge(plots[i]);
Plotly.purge(plots[i]);
}
},

Expand Down
7 changes: 4 additions & 3 deletions devtools/test_dashboard/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,22 +134,23 @@ function writeFilePromise(path, contents) {
function bundlePlotly() {
b.bundle(function(err) {
if(err) {
console.error('Error while bundling!', err);
console.error('Error while bundling!', JSON.stringify(String(err)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

before:

image

now:

image

} else {
console.log('Bundle updated at ' + new Date().toLocaleTimeString());
}

if(firstBundle) {
open('http://localhost:' + PORT + '/devtools/test_dashboard');
firstBundle = false;
}
console.log('Bundle updated at ' + new Date().toLocaleTimeString());
}).pipe(fs.createWriteStream(constants.pathToPlotlyBuild));
}

function bundleDevtools() {
return new Promise(function(resolve, reject) {
devtools.bundle(function(err) {
if(err) {
console.error('Error while bundling!', err);
console.error('Error while bundling!', JSON.stringify(String(err)));
return reject(err);
} else {
return resolve();
Expand Down
1 change: 1 addition & 0 deletions devtools/test_dashboard/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,5 @@ header span{
}
.dashboard-plot{
margin-bottom: 30px;
width: 1100px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

full window width default width deforms most plots. I think 1000px is more sensible. Of course this can always be overrode if desired.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's code in devtools.js that actually resizes the div.dashboard-plot when you search to fill the full width - I wasn't sure how best to deal with it and I figured most mocks would have dimensions set, but we may want to change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, most mock do have width set, but quick Plotly.plot(Tabs.fresh(), [{}]) don't and look ugly on the full window width.

}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@
"test-syntax": "node test/syntax_test.js",
"test-bundle": "node tasks/test_bundle.js",
"test": "npm run citest-jasmine && npm run test-image && npm run test-syntax && npm run test-bundle",
"start": "npm run start-test_dashboard",
"start-test_dashboard": "node devtools/test_dashboard/server.js",
"start-image_viewer": "node devtools/image_viewer/server.js",
"start": "npm run start-test_dashboard",
"baseline": "./tasks/baseline.sh",
"preversion": "npm-link-check && npm dedupe",
"version": "npm run build && git add -A dist src build",
Expand Down