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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mailchimp decorator enables corresponding category wizard and legends #3874

Merged
merged 5 commits into from
Jun 1, 2015

Conversation

alonsogarciapablo
Copy link
Contributor

Fixes #3826.

@Kartones can you please take a look? I added some inline comments. Thanks!

@@ -164,13 +164,23 @@ def get_presenter(options, configuration)
CartoDB::Layer::Presenter.new(self, options, configuration)
end

def set_style_options(cartocss_style)
def set_style_options(cartocss_style, opts = {})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These 3 methods are only being used by the MailchimpDecorator. I'd be happy to create a more generic method like set_option(name, value) and do something like layer.set_option('tile_style', cartocss) from the MailchimpDecorator. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

definetly yes!

I just added that as a first approach as current Layer model is quite barebones (as frontend JS is usually who controls layer data), so any addition is more than welcome!

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Kartones
Copy link
Contributor

Kartones commented Jun 1, 2015

👍

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor Author

Thanks @Kartones. Please take a look at this commit that introduces Layer#set_option to have a more generic way of setting options. Thanks!

alonsogarciapablo added a commit that referenced this pull request Jun 1, 2015
Mailchimp decorator enables corresponding category wizard and legends
@alonsogarciapablo alonsogarciapablo merged commit a081de0 into master Jun 1, 2015
@alonsogarciapablo alonsogarciapablo deleted the 3826-mailchimp-decorator branch June 1, 2015 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mailchimp decorator doesn't apply the styles on the category and it should
3 participants