Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Remove woocommerce hooks in cleanup.php.. #1082

Closed
JPOak opened this issue Sep 19, 2017 · 8 comments
Closed

Remove woocommerce hooks in cleanup.php.. #1082

JPOak opened this issue Sep 19, 2017 · 8 comments

Comments

@JPOak
Copy link
Contributor

JPOak commented Sep 19, 2017

With the way Woocommerce is currently used in the FoundationPress (woocommerce.php in the root), I don't think the hooks are needed in the cleanup.php .

I think it creates confusion when someone decides to use the more flexible hook method to integrate woocommerce it creates an error being there. In fact, technically with the way you currently have woo implemented you dont need to declare add_theme_support( 'woocommerce' ); .

Thoughts on removing this?

@colin-marshall
Copy link
Collaborator

@JPOak weren't you suggesting in #1057 to keep the hooks and drop woocommerce.php?

@JPOak
Copy link
Contributor Author

JPOak commented Sep 20, 2017

@colin-marshall Yes. But it is really one or the other. I prefer using the hooks, but if you want to go the woocommerce.php way in my opinion we should remove what is there in cleanup as it is not needed. If you want to go the woocommerce.php way I think that could just be changed around a bit to make the sidebar work as default and let the dev decide how they want to implement the sidebar or not.

@colin-marshall
Copy link
Collaborator

@JPOak thanks for clarifying. I personally would prefer to keep woocommerce.php because I'm not a big fan of writing markup inside of php functions, but that's just me.

@JPOak
Copy link
Contributor Author

JPOak commented Sep 20, 2017

@colin-marshall Ok. No worries. I still am not sure why there is the woocommerce reference in the cleanup, but as long as I know it is there I can remove as needed. I'll close this.

@JPOak JPOak closed this as completed Sep 20, 2017
@colin-marshall
Copy link
Collaborator

@JPOak sorry for not being clearer in my response. I am in favor of removing the unnecessary WooCommerce hooks in cleanup.php, and keeping woocommerce.php.

@JPOak
Copy link
Contributor Author

JPOak commented Sep 21, 2017

@colin-marshall ok. will send pull request.

@colin-marshall
Copy link
Collaborator

@JPOak thanks for the PR! So we can also remove the add_theme_support for WooCommerce if we're using woocommerce.php, correct?

@JPOak
Copy link
Contributor Author

JPOak commented Sep 21, 2017

@colin-marshall - yeah. I'll do a pr for that as well. What did you think of the refactored woocommerce.php here #1057 .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants