gambitph / Titan-Framework

The easiest to use WordPress option framework.
http://www.titanframework.net
377 stars 137 forks source link

Hide metaboxes on particular page templates #188

Open ivandotv opened 10 years ago

ivandotv commented 10 years ago

Referencing these issues #73 #7 Comming from Custom metaboxes , this feature is very handy, especially if there are multiple page templates, it is not good coding style to "hide" the metaboxes via javascript. I've looked at the code in the TitanFrameworkMetaBox register method. And I've tried to change it not to print the metabox if the page template is not the one I'm looking for. I've put this on top of the register method.

        $p             = get_post();
        $templateSlug  = get_page_template_slug( $p->ID );
        $pageTemplates = $this->settings['page_template'];
        if ( ! empty( $pageTemplates ) ) {
            if ( ! count( array_intersect( $pageTemplates, array( $templateSlug ) ) ) ) {

                return; // don't print the metabox
            }
        }

And creation of the metabox goes like this:

// Titan framework test
$titan       = TitanFramework::getInstance( 'my-theme' );
$postMetaBox = $titan->createMetaBox( array(
    'name'      => 'Additional Post Options',
    'post_type' => 'page',
    'page_template'=>array('team.php')
) );

If you think this is sufficient, I can make a pull request.

ardalann commented 10 years ago

The only problem with your approach is that when you change the template of a page using the drop-down, (before clicking on the Save button) nothing happens. The page is not reloaded so you still see the metaboxes of previous template and new metaboxes are not loaded. You will have to click on the Save button to see the new metaboxes. That's why it has to be a client-side solution. (Javascript)

ivandotv commented 10 years ago

Well I think that is implied. As far as I know, you can't generate metaboxes dynamically when the user changes the template. I think no one expects for the metaboxes to change when the template is changed. Creating all metaboxes for all page templates and then hiding them with javascript is not a good solution.

ardalann commented 10 years ago

You, as a developer, do not expect that. But normal users definitely do, except if you instruct them to press the Save button everytime they change the template. The other way to do this is using ajax to load new metaboxes and remove the old ones. I think that's an overkill though. Having hidden metaboxes increases page size and could make the page a bit slower in cheap mobile phones. But as long as it's only in the back-end, and only admins use it, I wouldn't mind it at all.

ivandotv commented 10 years ago

@ardalann What happens when you "save" the template with all those hidden metaboxes? I assume that they are all also saved to the database for that page id? And there is a scenario when some of the templates share the same metabox options. Using ajax and messing with the core 'add_meta_boxes' hook would create more problems/bugs that it solves.

ardalann commented 10 years ago

@ikixxx I agree that ajax is not a good solution for this. I think the same about opening a message popup. Redux uses the hiding solution.

If multiple templates share the same settings, that's totally fine. The new option that we add to createMetaBox (e.g. template) could be either a string (representing only one template) or an array of templates. About the fact that the values of all hidden metaboxes will be saved in the database... that's actually a good point. In big websites will thousands of posts and many post custom fields, having tons of "empty" meta data stored in the database is not ideal. In order to solve that, I think we can make the following change to class-titan-framework.php line #431 :

 else {
            // meta
            return update_post_meta( $postID, $this->optionNamespace . '_' . $optionName, $value );
        }

New code:

 else {
            // meta
            if ( empty($value) ) {
                return delete_post_meta( $postID, $this->optionNamespace . '_' . $optionName );
            } else {
                return update_post_meta( $postID, $this->optionNamespace . '_' . $optionName, $value );
            }
        }

What do you think @ikixxx , @bfintal , @oherman ?

ivandotv commented 10 years ago

@ardalann Yeah I was aware that popup is a bad solution as I was typing it.

Regarding the option for showing/hiding metaboxes, I think the logic is not too complicated. I'm guessing that the layout for metaboxes wrapper should be changed , so that they are connected to the template values (data attribute would be the best) data-page-template='templatename.php' And then just toggle the visibility of those wrappers on/off depending on the value of the dropdown.

ardalann commented 10 years ago

@ikixxx Yeah it should be straightforward. createMetaBox gets a new 'template' option. then if it's an array: $template = implode(',', $template) and prints it as a data attribute for tf-form-table (class-meta-box.php line 85) : data-template="$template" JS attaches an event to the template dropdown:

var current_template = $(this).val();
$('.tf-form-table').each(function(){
     var templates = $(this).data('template');
     if ( templates != '' ) {
          templates = templates.split(',');
          if ( templates.indexOf(current_template) == -1 ) {
                    $(this).parents('.postbox').addClass('tf-hidden-metabox');
          }  else {
                    $(this).parents('.postbox').removeClass('tf-hidden-metabox');
          }
     } 
});

I think it takes only a couple hours to add. But I don't know how long it'll take for bfinal and oherman to merge the pull request!

ivandotv commented 10 years ago

Nice work. One thing, there is no need to wrap the DOM element in jquery object every time you need to use it ($(this)) Do it only once var $this= $(this); and use that variable. It's a small performance optimization.

ardalann commented 10 years ago

@ikixxx You're right. I always do that when I use the jQuery obj of an element multiple times. I just thought it wasn't necessary here as

  1. It uses the jQuery obj once if the metabox doesn't have any specific templates, and only twice if it has.
  2. It's in the admin area, so it's not a big deal!
ardalann commented 10 years ago

A tiny improvement! :

var current_template = $(this).val();
$('.tf-form-table[data-template]').each(function(){
     var $this = $(this),
         templates = $this.data('template').split(',');
     if ( templates.indexOf(current_template) == -1 ) {
            $this.parents('.postbox').addClass('tf-hidden-metabox');
     }  else {
            $this.parents('.postbox').removeClass('tf-hidden-metabox');
     }
});
bfintal commented 9 years ago

PRs welcome :+1:

ivandotv commented 9 years ago

I was thinking about this. Basically if all metaboxes need to be printed, then there should be a script that immediately removes all other boxes not connected with the template out of the DOM, but it doesn't deletes them. The scripts listens to the select event on the template drop down, and if the value is changed , it appends the correct metaboxes. I'll see if I can make something work when I get some free time.

bfintal commented 9 years ago

@ikixxx Yea!

Maybe a good idea also to add a new parameter called template in https://github.com/gambitph/Titan-Framework/blob/master/class-meta-box.php http://www.titanframework.net/meta-box/ for this.

So that your script would only run when template is present, and we can use that value also for checking whether to show the meta box or not. Maybe a good idea also would be to only trigger the hide/show the meta boxes that use template

If template isn't used or if it's blank, then the meta box would basically be for any template - the current behavior.

mrhammadasif commented 8 years ago

Any Update on this, If the team is going to implement the template option or not ?

banna360 commented 5 years ago

We are waiting for this feature. Large site have lot of option in different pages. But we don't need show meta box of a different page in all pages.