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

ZERO circular dependency #2429

Merged
merged 17 commits into from
Mar 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 1 addition & 2 deletions src/components/modebar/buttons.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ var Registry = require('../../registry');
var Plots = require('../../plots/plots');
var axisIds = require('../../plots/cartesian/axis_ids');
var Lib = require('../../lib');
var downloadImage = require('../../snapshot/download');
var Icons = require('../../../build/ploticon');

var _ = Lib._;
Expand Down Expand Up @@ -60,7 +59,7 @@ modeBarButtons.toImage = {
format = 'svg';
}

downloadImage(gd, {'format': format})
Registry.call('downloadImage', gd, {'format': format})
.then(function(filename) {
Lib.notifier(_(gd, 'Snapshot succeeded') + ' - ' + filename, 'long');
})
Expand Down
6 changes: 3 additions & 3 deletions src/plot_api/to_image.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

'use strict';

var Registry = require('../registry');
var plotApi = require('./plot_api');
var Lib = require('../lib');

var helpers = require('../snapshot/helpers');
Expand Down Expand Up @@ -155,7 +155,7 @@ function toImage(gd, opts) {
var width = clonedGd._fullLayout.width;
var height = clonedGd._fullLayout.height;

Registry.call('purge', [clonedGd]);
plotApi.purge(clonedGd);
document.body.removeChild(clonedGd);

if(format === 'svg') {
Expand Down Expand Up @@ -196,7 +196,7 @@ function toImage(gd, opts) {
}

return new Promise(function(resolve, reject) {
Registry.call('plot', [clonedGd, data, layoutImage, configImage])
plotApi.plot(clonedGd, data, layoutImage, configImage)
Copy link
Collaborator

Choose a reason for hiding this comment

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

just curious, why did you choose not to use Registry here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plotly.toImage shouldn't be in a circular dep with the other Plotly methods. Plotly.toImage should call Plotly.plot but never the other way around.

Ideally, I think the api method registry should only be used in components/ and in the subplot modules (for interaction updates). Plotly methods need to call component methods (e.g .draw) and some components need to call Plotly methods (e.g. updatemenus / mode bar button click) - this is circular, hence the need for an api method registry.

.then(redrawFunc)
.then(wait)
.then(convert)
Expand Down