Skip to content
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

Remove fails on group field if no TinyMCE present on the page #730

Closed
rogercoathup opened this issue Sep 9, 2016 · 9 comments
Closed

Remove fails on group field if no TinyMCE present on the page #730

rogercoathup opened this issue Sep 9, 2016 · 9 comments

Comments

@rogercoathup
Copy link

Issue:

I have a CPT which is just title, excerpt and a CMB metabox containing a repeatable group field.

In this situation, remove doesn't work on group field -- throws a JS error in cmb2.min.js:

Line 382 ReferenceError: Can't find variable: tinyMCE

With a WYSIWYG editor present on the CPT, remove on the group field works as expected.

CMB2 Field Registration Code:

    $args = array(
            'labels' => $labels,
            'public' => true,
            'publicly_queryable' => true,
            'show_ui' => true,
            'query_var' => true,
            'rewrite' => array('slug' => 'slider'),
            'capability_type' => 'post',
            'hierarchical' => false,
            'menu_icon' => 'dashicons-images-alt',
            'has_archive' => true,
            'supports' => array( 'title', 'excerpt', 'thumbnail'),
            'taxonomies' => array()
        );

    register_post_type( 'a21_slider' , $args );

....
    $prefix = '_a21_slider_';

    $slides_box = new_cmb2_box( array(
        'id' => $prefix . 'slides',
        'title' => __( 'Slides', 'a21-slider' ),
        'object_types' => array( 'a21_slider' ),
        'context' => 'normal',
        'priority' => 'default',
        'show_names' => true
      ));

    $slides_group = $slides_box->add_field( array(
        'id'          => $prefix . 'slides',
        'type'        => 'group',
        'description' => __( 'Select your slides', 'a21-slider' ),
        'options'     => array(
            'group_title'   => __( 'Slide {#}', 'a21-slider' ),
            'add_button'    => __( 'Add Another Slide', 'a21-slider' ),
            'remove_button' => __( 'Remove Slide', 'cmb2' ),
            'sortable'      => true,
        ),
    ) );


    $slides_box->add_group_field( $slides_group, array(
            'name' => __( 'Slide', 'a21-slider' ),
            'id'   => $prefix . 'slide',
            'type' => 'file'
        ) );
}
@jtsternberg
Copy link
Member

I'm not able to replicate this issue. Have you tested on the trunk branch?

@rogercoathup
Copy link
Author

Using the following branch: webdevstudios/cmb2 dev-trunk 17500e9 CMB2

@jtsternberg
Copy link
Member

Ok, interesting. Well, as I stated, I'm not able to replicate. Not sure what I else I can do to test.

@decapoda
Copy link

decapoda commented Oct 26, 2016

Had similar thing on pages with custom post types that didn't have any tinyMCE fields. When trying to remove a repeatable item had that:

Uncaught ReferenceError: tinyMCE is not defined - cmb2.min.js?ver=2.2.3:1

Highlighted spot:
function(id){var editor=tinyMCE.get(id);...

I assume, the problem is inside line 298 in cmb2-wysiwyg.js, which is shipped in cmb2.min.js.

Fixed with loading tinymce (for those custom posts post.php, edit.php and post-new.php admin pages):

wp_enqueue_script('tinymce_js', includes_url('js/tinymce/') . 'tinymce.min.js');

@tw2113
Copy link
Contributor

tw2113 commented Oct 26, 2016

@decapoda you should be able to just do wp_enqueue_script( 'tiny_mce' ); from the looks of the wp_enqueue_script developer page.

https://developer.wordpress.org/reference/functions/wp_enqueue_script/

@jtsternberg perhaps we need to add tiny_mce as a conditional dependency?

@decapoda
Copy link

@tw2113 Yep, tried it both ways (as wp_enqueue_script( 'tiny_mce' ); and as a dependency to my other script for my custom posts admin pages), didn't work

@jtsternberg
Copy link
Member

This should be good. Please test the trunk branch. Unfortunately I just pushed a release today, so will be a bit before this is released (unless I get a lot of reports of this issue). But you should be safe to use trunk as the only difference from master is this change.

@decapoda
Copy link

Yep, it's all good with the trunk version, thanks @jtsternberg

@tw2113
Copy link
Contributor

tw2113 commented Oct 27, 2016

Awesome to hear.

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

No branches or pull requests

4 participants