-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Allow transform to have supply layout defaults handler #1122
Conversation
- and pass it to supplyLayoutModuleDefaults
- which is now assumed to be in fullLayout
_module = transformModules[i]; | ||
|
||
if(_module.supplyLayoutDefaults) { | ||
_module.supplyLayoutDefaults(layoutIn, layoutOut, fullData, transitionData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rreusser I chose to only pass transitionData
to transform module supplyLayoutDefaults
, because that's the only place where we need them at the moment.
Looks good to me. Only exception is in plot_api/register.js. Can we add |
By check, you mean something like: if(typeof newModule.supplyLayoutDefaults !== 'function') {
throw new Error('supplyLayoutDefaults method is not a function');
} ? Otherwise, |
I was referring to the fact that a transform that only has supplyLayoutDefaults (as this one does) fails unless it has a dummy no-op for either |
I would prefer keeping this as such. Transform modules should transform the data using |
Agreed, though the current use-case is an exception to that rule. I give it the 💃 though. |
🎉 thanks! |
a requirement for #1099
cc @rreusser