-
Notifications
You must be signed in to change notification settings - Fork 44
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
CATALOG-2408 - Allow template property for optionChange #79
Conversation
Autotagging @bigcommerce/storefront-team |
src/api/product-attributes.js
Outdated
@@ -21,9 +21,9 @@ export default class extends Base | |||
* @param {Object} params | |||
* @param callback | |||
*/ | |||
optionChange(productId, params, callback) { | |||
optionChange(productId, params, template=null, callback) { |
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.
I'd do something like
if (typeof template === 'function') {
let temp = callback;
callback = template;
template = temp;
}
immediately at the top of the function. That gives you a modicum of backwards compatibility.
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.
Added! Gonna test it now...
4ed83ba
to
573aa1f
Compare
bigcommerce/cornerstone#1167 related cornerstone PR |
25396da
to
7693d8f
Compare
@lord2800 linter is happy, re-review? |
I confirmed this doesn't break existing Cornerstone if I use it without any code changes in the theme, so I believe it to be backwards compatible. |
7693d8f
to
d30177d
Compare
d30177d
to
584d061
Compare
I have a use case where I need to return part of the product options payload as rendered HTML. Therefore, I need to be able to send a template property.