search

pootlepress / sfx-page-customizer

Page Customizer for Storefront
0 stars 0 forks source link

Feedback from WooThemes... [IMPORTANT!] #44

Closed nickburne closed 9 years ago

nickburne commented 9 years ago
  1. For the hide options, I'd really prefer to actually remove elements rather than hiding them with css. I think they're all just hooked in so that should be simple enough.
  2. Remember the breadcrumbs are only added when WooCommerce is active. So best to check for WooCommerce before displaying that setting.
  3. Please ensure that all data is sanitized / escaped where appropriate and that wp coding standards are adhered to throughout.
  4. I think we should change 'Storefront settings' in the meta box to something more relevant.
nickburne commented 9 years ago

This is from James @Woothemes - very experienced woothemes/woocommerce developer we really need to impress

What are your thoughts?

shramee commented 9 years ago

Hmm... Yeah I agree with all the points.

nickburne commented 9 years ago

For number 3, do you want to do a review of your own code. I am very interested in this feedback...

shramee commented 9 years ago

Yeah, I'll just go through it and for the last point, may be we should keep the name something like 'Page settings' or 'Page Customizer' rather than Storefront settings.

nickburne commented 9 years ago

Let's call it 'Customize Storefront options for this page'

nickburne commented 9 years ago

I am off for today, will check in with you tomorrow!

shramee commented 9 years ago

Okay...

shramee commented 9 years ago

Done, I did breadcrumbs stuff with Header cart also.

nickburne commented 9 years ago

Can you tell me more about this one "Please ensure that all data is sanitized / escaped where appropriate and that wp coding standards are adhered to throughout."?

Thanks!

shramee commented 9 years ago

Yep, Sorry for the delay, was in a meeting.

shramee commented 9 years ago

The data stored in WP from our meta boxes is saved by our function hooked to save_posts action. However any data saved via update_post_meta is already escaped by wp, still I just added esc_url_raw() for images and sanitize_text_field() for other fields.

nickburne commented 9 years ago

Need to talk you about this next week @DuncanChittick