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

Include scripts should be conditionally based on fields used #136

Closed
GregLancaster71 opened this issue Jan 7, 2015 · 5 comments
Closed

Comments

@GregLancaster71
Copy link
Contributor

CMB2 Loads an immense amount of inline script tags when used with front end forms. Even when none are necessary. Im using it to generate a simple review form, and WOW - Thats a lot of inline scripts. Is there a way to disable the inclusion of the wordpress media template? Its kinda huge, and is unnecessary,

@GregLancaster71
Copy link
Contributor Author

Edit: For now I wrapped line 221 in helper-functions.php with if ( is_admin() ) {}. Now before I upload this to a live site, is there any reason this would break anything?

@jtsternberg
Copy link
Member

Yes, don't edit the core files, or you'll break it for other plugins/themes that are depending on the library. Are you sure you mean line 221? https://github.com/WebDevStudios/CMB2/blob/master/includes/helper-functions.php#L221
Also, can you be more specific about which scripts are loaded? I agree, this should not happen unless the field types you add need them.

@jtsternberg jtsternberg changed the title Inline Scripts on Front End Include scripts should be conditionally based on fields used Jan 7, 2015
jtsternberg added a commit that referenced this issue Jan 7, 2015
@GregLancaster71
Copy link
Contributor Author

My Mistake - Its this line : CMB2_hookup::enqueue_cmb_js();.

Additionally: Thank you making this guys. I love it, and it makes my life so much easier.

@jtsternberg
Copy link
Member

I just added a simple text field to the front-end, and yah, I got the whole WordPress media setup enqueued. Definitely a bug. But again, please do not edit the core files. It is a bad idea for many reasons. I've included a filter for the JS and the CSS enqueueing so you can at least block them that way.

function maybe_not_enqueue_cmb_js( $do_enqueue ) {

    /**
     * Make sure to set a limited scope for your condition so you don't
     * disable JS for every plugin/theme that may use this feature.
     */
    $my_conditions_are_matched = ( ! is_admin() && 'custom-post-type' == get_post_type() ); 

    if ( $my_conditions_are_matched ) {
        // Ok, then, don't enqueue the JS
        return false;
    }

    return $do_enqueue;
}
cmb2_enqueue_cmb_jsadd_filter( 'cmb2_enqueue_js', 'maybe_not_enqueue_cmb_js' );

We'll need to work on making these script enqueues more conditional in the future.

@AlchemyUnited
Copy link
Contributor

@jtsternberg - I can make a go at this if you can prehaps fill me in with where to start, where to look, etc. That said, while I understand it's gonna create a larger file, can't all the js just be jammed together? swallow that once and then not have to worry about it? I was just reading someone's article and basically it came down to: splitting hairs didn't beat just one larger payload one time. Unless of course we're talking about something massive.

That said...

Is there anywhere that maps form component to js files required for that component? that would make sense (I think). Then it's just a matter of saying "what's on this page" and "okay then, enqueue these (and not these)." at least that's how I think I would want to approach it.

fwiw, I do my enqueing with arrays. That is, I have all the details for each script or CSS in an array, and then I run that thought a method that does the actual enqueueing. The point is, that kinda makes manipulation of that easier since the enqueue'ing is done programatically and not hardcoded. One of the properties / fields in my array is active. It's a simple bool switch for turning stuff on and off. Yeah, I've run into this kinda thing before ;)

LMK

Thx
mfs

pluginmirror-worker pushed a commit to wp-plugins/cmb2 that referenced this issue May 29, 2015
* Ability to use non-repeatable group fields by setting the `'repeatable'` field param to `false` when registering a group field type. Props [marcusbattle](https://github.com/marcusbattle), ([#159](CMB2/CMB2#159)).
* Add and enqeueue a front-end specific CSS file which adds additional styles which are typically covered by wp-admin css. ([#311](CMB2/CMB2#311))
* Better handling of the CMB2 javascript (and CSS) required dependencies array. Dependencies are now only added conditionally based on the field types that are actually visible. ([#136](CMB2/CMB2#136))
* **THIS IS A BREAKING CHANGE:** The `group` field type's `'show_on_cb'` property now receives the `CMB2_Field` object instance as an argument instead of the `CMB2` instance. If you're using the `'show_on_cb'` property for a `group` field, please adjust accordingly. _note: you can still retrieve the `CMB2` instance via the `cmb2_get_metabox` helper function._
* New dynamic hook, `"cmb2_save_{$object_type}_fields_{$this->cmb_id}"`, to complement the existing `"cmb2_save_{$object_type}_fields"` hook.
* New CMB2 property, `enqueue_js`, to disable the enqueueing of the CMB2 Javascript.
* German translation provided by Friedhelm Jost.

### Bug Fixes

* Fix incorrect repeatable group title number. ([#310](CMB2/CMB2#310))
* Fix obscure bug which prevented group field arguments from being passed to the sub-fields (like `show_names` and `context`).
* Fixed occasional issue when adding a group row, the previous row's content would be cloned. ([#257](CMB2/CMB2#257))


git-svn-id: https://plugins.svn.wordpress.org/cmb2/trunk@1169742 b8457f37-d9ea-0310-8a92-e5e31aec5664
pluginmirror-worker pushed a commit to wp-plugins/cmb2 that referenced this issue May 29, 2015
* Ability to use non-repeatable group fields by setting the `'repeatable'` field param to `false` when registering a group field type. Props [marcusbattle](https://github.com/marcusbattle), ([#159](CMB2/CMB2#159)).
* Add and enqeueue a front-end specific CSS file which adds additional styles which are typically covered by wp-admin css. ([#311](CMB2/CMB2#311))
* Better handling of the CMB2 javascript (and CSS) required dependencies array. Dependencies are now only added conditionally based on the field types that are actually visible. ([#136](CMB2/CMB2#136))
* **THIS IS A BREAKING CHANGE:** The `group` field type's `'show_on_cb'` property now receives the `CMB2_Field` object instance as an argument instead of the `CMB2` instance. If you're using the `'show_on_cb'` property for a `group` field, please adjust accordingly. _note: you can still retrieve the `CMB2` instance via the `cmb2_get_metabox` helper function._
* New dynamic hook, `"cmb2_save_{$object_type}_fields_{$this->cmb_id}"`, to complement the existing `"cmb2_save_{$object_type}_fields"` hook.
* New CMB2 property, `enqueue_js`, to disable the enqueueing of the CMB2 Javascript.
* German translation provided by Friedhelm Jost.

### Bug Fixes

* Fix incorrect repeatable group title number. ([#310](CMB2/CMB2#310))
* Fix obscure bug which prevented group field arguments from being passed to the sub-fields (like `show_names` and `context`).
* Fixed occasional issue when adding a group row, the previous row's content would be cloned. ([#257](CMB2/CMB2#257))


git-svn-id: https://plugins.svn.wordpress.org/cmb2/trunk@1169744 b8457f37-d9ea-0310-8a92-e5e31aec5664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants