Make Plotly.plot agnostic to plot modules #160
Conversation
instead of hard-coding the module names
|
In short: This PR makes the |
|
|
||
| exports.supplyLayoutDefaults = require('./layout/defaults'); | ||
|
|
||
| exports.plot = function plotGeo(gd) { |
There was a problem hiding this comment.
e.g. the geo plotting routine accessed in Plotly.plot via the plots registry.
| exports.Scene2D = require('./plots/gl2d/scene2d'); | ||
|
|
||
| var Geo = require('./plots/geo'); | ||
| Plots.registerSubplot(Geo); |
There was a problem hiding this comment.
one step towards modularity. This register step will taken out of the core module and added on-demand by users.
|
I can't see anything that looks out of place, but I'd like to give it another read through tomorrow (and dig deeper into the existing way of doing things). Looks like we're moving in the right direction though 👍 |
- assume that if fullLayout._has[] is set, the corresponding plot module has been registered. If not the case, this will result in an error.
instead of relying an a hard-coded list.
|
@mdtusz one last look? |
| }, | ||
| fullLayout | ||
| ); | ||
|
|
There was a problem hiding this comment.
What's the style preference here - to have the ); aligned with the initial code (scene) or on the end of fullLayout?
There was a problem hiding this comment.
aliged with the initial code (scene)
looks better to my eyes. But I have no strong opinion.
|
💃 after minor style things. |
Make Plotly.plot agnostic to plot modules
Checks off a few bullet items in #41 (comment)
Not sure, if it's worth merging before @mdtusz 's module registry lands (ref #157), but here's a glimpse of things to come.