support multiple generic marketing modals #119
Conversation
Generated by 🚫 danger |
| modalData = _.findWhere(sd.MARKETING_SIGNUP_MODALS, { slug: slug }) | ||
|
|
||
| if modalData | ||
| sd.MARKETING_SIGNUP_MODAL_IMG = modalData.image |
There was a problem hiding this comment.
This is a clever way to inject data into sd to leverage the existing code, but be careful because sd is a global object. On the server that means it's shared across requests and can cause unexpected behaviors, or worse, leak user-specific data across requests if you're not careful.
You could address this by simply modifying the request level sd via res.locals.sd. MARKETING_SIGNUP_MODAL_IMG = ... or refactor the code downstream so that you're passing the modal data through locals e.g. res.locals.modal = modalData and changing the template to use modal.image modal.copy etc. to be more like the refactor we did in Force.
Either works for me, but the latter might be nicer as it's more consistent with what we do in Force.
|
Thanks for the refactors, looks great! |
closes: https://github.com/artsy/collector-experience/issues/121
This is the mg equivalent for artsy/force#826 and adds support for multiple marketing campaign modals using a hardcoded stringified JSON object set in heroku.