lucidnz / bootstrapify-1

An open-source base theme for Shopify using Twitter Bootstrap
http://bootstrapify-theme.myshopify.com/
187 stars 72 forks source link

Closes #186 - Use Grunt plugin to manage theme settings. #205

Closed gavinballard closed 9 years ago

gavinballard commented 9 years ago

Hi guys, here's my implementation of the grunt-shopify-theme-settings Grunt plugin for Bootstrapify. Everything got ported across fairly smoothly, the exceptions being some of the funkier box layouts you were using in your settings HTML got lost. Hopefully the convenience of the YAML syntax is worth it :).

Here a couple of screenshots of the settings panel before the changes and after the changes, so you can cast your eyes over it.

I also did a double-check that no settings were lost or changed names by doing a diff on the output of $('#theme_settings_form').serializeArray() from the Javascript console in Chrome when viewing the before/after settings.

Let me know what you think, looking forward to any feedback you might have!

stewartknapman commented 9 years ago

Hey @gavinballard I've cast my eyes over it super (super) briefly and it's looking cool. But I'll have a proper look later today.

We could probably recreate the font and slide setting groups as templates. Whoa, what would the possibility be of including a custom templates folder for the plugin? i.e. would sit in settings dir and the plugin would use that before the standard ones if they exist?

Also now I'm just being curious, how come you used swig instead of a node liquid port like tinyliquid?

stewartknapman commented 9 years ago

Ahh @gavinballard also, just to be a pain we've added a couple of new settings that I don't think you've got. I'm working on them now so I'll let you know when we are done and they are in master.

stewartknapman commented 9 years ago

@gavinballard I'm pretty sure we are good for any new settings now. I've asked the others (/cc @darryn @galenking ) to hold off from adding any new settings.

gavinballard commented 9 years ago

Thanks Stewart, will rebase today and try to pick up those additional settings. The custom templates things probably isn't too difficult either, I'll have a think... would definitely be a cool / useful feature.

Re: Swig, I initially tried to get things done with grunt-liquid, but from memory was running into quite a few issues with things like iterating over objects and missing a few useful features. Swapping to Swig got things going pretty quick so I didn't come back to it. I don't think I've checked out tinyliquid, so I'll have a look at that!

gavinballard commented 9 years ago

Hey @stewartknapman, have updated this PR to take into account the new settings.

Have had a quick play around with the templates, and I think I have a potential solution, but there are a couple of tricky aspects (like Swig not supporting multiple template include paths) that I don't have time right now to sort out. I can probably get something together this week though.

stewartknapman commented 9 years ago

Awesome, thanks @gavinballard. I'll have a look when I get in this morning.

stewartknapman commented 9 years ago

Thanks @gavinballard

gavinballard commented 9 years ago

No worries @stewartknapman. Let me know if you see any issues cropping up. I think the templates idea is pretty useful, too, so will ping you when I get around to implementing that.

stewartknapman commented 9 years ago

@gavinballard sounds good.