search

spree-contrib / spree_contact_us

Adds Contact Us form to your Spree Commerce store
https://guides.spreecommerce.org
BSD 3-Clause "New" or "Revised" License
40 stars 138 forks source link

Fixed broken link and updated Usage #51

Closed kinsomicrote closed 9 years ago

kinsomicrote commented 9 years ago

This link https://github.com/spree-contrib/contact_us points to no where. It has been removed now. Also a little heads-up about using the gem and a link to a recommended guide has been added.

JDutil commented 9 years ago

Thanks. Certainly would be good to update markup for bootstrap now to use default styles skeleton didn't used to provide.

kinsomicrote commented 9 years ago

Would that be possible? I don't think there is a style for contact-us to inherit as it comes with it's own class - 'not' in the override What i did was to use the styles used on Home link found here

JDutil commented 9 years ago

Yes you add the bootstrap classes http://getbootstrap.com/css/#forms

jspizziri commented 9 years ago

I thought I had added all of the bootstrap classes, I must've missed something.

JDutil commented 9 years ago

@jspizziri looks like you've done them perhaps @kinsomicrote is not fully up to date or the .form-group class is needed on the p tags but I thought just .form-control on the inputs would do.

kinsomicrote commented 9 years ago

I am not talking about about the forms. Of course they are okay. The update is about the contact us link added to the navbar.

kinsomicrote commented 9 years ago

@JDutil @jspizziri Am I on point?

jspizziri commented 9 years ago

It looks like that nav element is being inserted in slightly the wrong place as it ought to be inside of the ul. That is likely what is causing the styling issue that you're seeing.

If you look here https://github.com/spree/spree/blob/master/frontend/app/views/spree/shared/_main_nav_bar.html.erb you'll see that #main-nav-bar is a div and not a ul.

Do you want to submit a PR for this?

kinsomicrote commented 9 years ago

I don't have a fix for this yet. Would work on it tonight and see what I can come up with.

jspizziri commented 9 years ago

I can do it, it'll only take a few seconds.

jspizziri commented 9 years ago

This commit should fix your issue: https://github.com/spree-contrib/spree_contact_us/commit/83e2b69173c8d32a91dcaeee3d07a942174dde44

@JDutil, I committed this to master and then I realized I probably should've made a PR so you could look at it. Feel free to revert the change if you don't like it.