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

Usability #1

Closed dandyhat closed 11 years ago

dandyhat commented 12 years ago

Hi

Just tried out the gem. Great work. :) Just wanted to make note of some UI notes. 1) a red star does not appear next to required fields.

2) The error message when leaving fields blank is: 3 errors prohibited this record from being saved: There were problems with the following fields: Email is invalid Email can't be blank Name can't be blank

This could be made more friendly for non-technical users. Perhaps: Your message could not be sent due to: Email field was left blank Name field was left blank

3) How about a letting javascript produce the error instead of another server request?

JDutil commented 12 years ago

Thanks I'm glad you've found it useful.

1) I must have missed the red star indicating the required fields... I'll definitely want those.

2) As for the error messages I agree they could be more useful. For now I had simply let them display in the generic Spree way. Will have to look at modifying these with the i18n locales I think should do the trick.

3) This would be nice, but I'm hesitant to add a new dependency to accomplish it. Are you familiar with if theres a nice way to do this already with just Spree Core's javascript assets?

All in all thanks for the suggestions I'll definitely try to get 1 & 2 done, but will need to look into the best way to handle 3. If you are up for submitting a pull request for any of this stuff before I can get around to it that would be awesome.

dandyhat commented 12 years ago

:D

4) Add the ability to leave a phone number. Some people may want this.

re 3: best to ask the mailing list.

Thanks for the extension.

dandyhat commented 12 years ago

Should the star isn't red. Should this be handled by the theme or the extension?

JDutil commented 12 years ago

The base Spree theme will color the star red once this commit makes it into the version your app uses: https://github.com/spree/spree/commit/9428de3c33de38f95fa14b100f4a64285cad8781

Until then you will want to style the field with your own theme.

dandyhat commented 12 years ago

Sounds great.

mez commented 12 years ago

What about re-ordering the contact us link on the header? How can I make it as the last link? I have spree_static_content extension. Maybe with a deface on the header override? Any help would be great!

JDutil commented 12 years ago

Yes you will need to write a custom deface override.

mez commented 12 years ago

I checked out the override that is there. The selector used is insert_bottom, why does it not play nice with spree_static_content? It is inserting it right after the Home link?

JDutil commented 12 years ago

It will depend on what order you are adding the extensions to your gemfile. If spree_static_content is required after spree_contact_us then all static content pages would be placed after the contact us link. If you place spree_contact_us second in the Gemfile it would move the link to after the static content page ones. If you have conflicts with other extension overrides you'll either need to modify your Gemfile order again or create a custom override to ensure contact us link always is last.

mez commented 12 years ago

Thank you so much for the quick response. Your suggestion worked! I did not think the gem order would matter. Any docs I can read that can explain to me why this is so?

JDutil commented 12 years ago

Well the reasoning is that the overrides are loaded in the order the extensions are loaded, which is dependent on the order of the Gemfile.

I thought I had seen reference to this in either the Deface README or the Spree Guides" https://github.com/railsdog/deface http://guides.spreecommerce.com/view_customization.html

A quick look through them didn't turn up mention of it though. This would be useful to document for sure if it wasn't already.

JDutil commented 12 years ago

Actually there is a brief message about it in this doc, but it doesn't specify it's due to the override loading order: http://guides.spreecommerce.com/extensions.html