-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add meta boxes on "add_meta_boxes" hook, not "init" #4
Comments
@tollmanz, this is quite tricky actually. Your thinking is correct but take the Anyhow, moving the actual metaboxes creation (not the whole class) to |
@kovshenin...Yeah, I get that "get_post_option" won't work if not added early on. I've not been using it because I don't see how it's any better than "get_post_meta" at this point. I can see that down the line, this could be a more powerful and useful function and it would be good to be forward thinking about how it would be implemented. Looking at this now, I think that the only method in the "Post_Options_API_1_0_1" that would need to run earlier than "add_meta_boxes" is the "_save_post" method (but I'm not entirely sure about that). My main concern is that on something like a front end request, the "Post_Options_API_1_0_1" class does not need to be instantiated. I think it would be good form to relegate it to only admin requests, and even better, to only situations when meta boxes are needed. |
@tollmanz, I ran a quick benchmark, I started a Anyhow, I do understand your concerns and it's probably a matter of design and code quality rather than performance (at least at this stage). What do you think of introducing an action, like Needs to be thought through and tested because of the current compatibility scheme. We don't want two different bundled version to register options twice, right? :) |
@kovshenin, would love to see what you have in mind. Unfortunately, I not sure how you would implement the lazy load option, but would like to see it in action. |
This might be a bigger issue than I thought. I'm seeing a nasty side effect of initiating the meta box functions on Initially, I had 8 fields in one section. Everything worked fine. Once I added a second section with one field, this error began. This is just a place that these functions should not run. I'm going to look into a way of changing this. |
@tollmanz, can I take a look at the code that you're using and what is your memory limit in php.ini? Perhaps I'm missing some "cleanup" scenario but I don't think it's related to |
@kovshenin, sorry for the late reply. Unfortunately, I've moved past this issue and don't think I have the code anymore. I'll try to dig it up. I agree that the issue will likely still occur with those hooks, but again, shouldn't we be loaded the library only when necessary. The site I was working on needs to perform really well. It's a problem if the library is getting loaded on every page view even when it's not needed. I understand that parts of the class need to be loaded at other times, but I just am not liking the overhead of loading this library on every request. |
@tollmanz, late reply me too, yeah I know :) I'll first sort out the memory problem and then look for better place to load up the stuff. Any luck finding that code again? P.S. What is the memory limit set to on your setup? |
I'm working with my second project utilizing post options API. On both projects, I called the function that initiates the post options API class from the "add_meta_boxes" hook. It did not work. Calling from "init" did.
The issue I have with this is that the "add_meta_boxes" hook was intended for the purpose of adding meta boxes. Since this project adds meta boxes, I think that it should work on this hook. Additionally, if the function that calls the post options API class methods is attached to the "add_meta_boxes" hook, that code will only be evaluated in the context of situations where meta boxes are needed. Using "init" forces WordPress to handle this code on each request.
Currently, I cannot figure out why "add_meta_boxes" won't work. I'm going to look into it more. Any thoughts?
The text was updated successfully, but these errors were encountered: