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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add registry of component modules #845

Merged
merged 18 commits into from Aug 19, 2016
Merged

Add registry of component modules #845

merged 18 commits into from Aug 19, 2016

Conversation

etpinard
Copy link
Contributor

This PR introduces a new registry, this one for component modules. For those keeping track, we now have registries for trace, plot (aka subplot aka base plot), transform and component modules.

This new component registry, will allow us to remove (some, maybe all?) component modules from the core plotly.js module for 馃崈 custom builds in v2.0.0.

Internally, this PR

  • pulls out Plotly.register out of src/plotly.js and into src/plot_api/register.js side-by-side with its Plotly method friends
  • pulls out all the registry-related objects and methods out of src/plots/plots.js and into src/registry.js. This new registry.js module only depends on Lib and one attribute file which allow us to 馃敧 a lot of circular dependencies
  • adds a helper method Register.getComponentMethod used to access method of registered component modules without having to require the module themselves.

- this will help free Plots of some circular dependencies
- make sure the ref back Registry contents in Plots
  for backward compatibility
by:
- requiring svgTextUtils in plot_api.js
- moving a injected modules in to src/core.js
- requiring some module directly in src/core.js
  instead of through src/plotly.js
- legend, annotations, shapes, images, rangeslider, rangeselector
  and updatemenus are now registerd in src/core.js
- add 'moduleType' and 'name' properties to component modules
- rename 'attributes' prop -> 'layoutAttributes' (for consistency)
- rename 'supplyLayoutDefaults' -> 'handleDefaults' when component
  defaults are required in another default module
- so that they are called only when the range slider/selector
  components are registered
Legend.draw(gd);
RangeSelector.draw(gd);
UpdateMenus.draw(gd);
Registry.getComponentMethod('legend', 'draw')(gd);
Copy link
Contributor Author

@etpinard etpinard Aug 11, 2016

Choose a reason for hiding this comment

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

Unlike traces and plot modules, we can't simply loop over them in a given plot step. Some components may require two draw calls per plot call, some component may not. Some components may have a calc step, some components may not etc.

So instead, we use a getter method to the component method on-demand. Note that if a given component isn't register, Registry.getComponentMethod() will return a noop.

@@ -498,6 +493,10 @@ module.exports = {
].join(' ')
},

_nestedModules: {
Copy link
Contributor Author

@etpinard etpinard Aug 11, 2016

Choose a reason for hiding this comment

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

this tells Plotly.PlotSchema.get to look for the attributes in the component registry.

@mdtusz
Copy link
Contributor

mdtusz commented Aug 18, 2016

Okey dokes. After the conflicts are all bueno and lil change to the extend then 馃拑 away. Woop woop less circular deps and plugins (of a sort).

@etpinard etpinard merged commit 7e4d8ab into master Aug 19, 2016
@etpinard etpinard deleted the registry-use-component branch August 19, 2016 13:28
@etpinard etpinard mentioned this pull request Aug 23, 2016
14 tasks
etpinard added a commit that referenced this pull request Aug 24, 2016
- use 'drawOne' mehtod instead of 'draw' as per PR #833
- use Registry instead of Plotly[module] as per PR #845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants