jcchavezs / cmb2-conditionals

Plugin to relate fields in a CMB2 metabox
GNU General Public License v2.0
86 stars 61 forks source link

Didn't work on Group Field #2

Open fujianto opened 9 years ago

fujianto commented 9 years ago

Hello,

First, thanks for the great addon. A real life saver on my current project. I've tried to use it on Group Field but it didn't work. Here's my code, hope you can tell me which mistakes I've made.

      $ads_box->add_field( array(
        'name'    => __('Preroll Video Sources' , 'orangerdev-base'),
        'id'      => $prefix . 'preroll_sources',
        'type'    => 'radio_inline',
        'default' => '',
        'options' => array(
            'youtube'         => __( 'Youtube'     , 'vidrev-ads' ),
            'vimeo'           => __( 'Vimeo'       , 'vidrev-ads' ),
            'dailymotion'     => __( 'Dailymotion' , 'vidrev-ads' ),
            'gdrive'          => __( 'Google Drive', 'vidrev-ads' ),
            'selfhosted'      => __( 'Self Hosted' , 'vidrev-ads' ),
            ),
            'attributes' => array(
                'required' => true, // Will be required only if visible.
                'data-conditional-id' => $prefix . 'campaign_type',
                'data-conditional-value' => 'preroll',
            )
        ));

    $selfhosted_sources_group = $ads_box->add_field( array(
            'id'          => $prefix .'selfhosted_sources',
            'type'        => 'group',
            'description' => __( 'Self Hosted Preroll Video', 'vidrev-ads' ),
            'options'     => array(
                'group_title'   => __( 'Video {#}', 'vidrev-ads' ), // since version 1.1.4, {#} gets replaced by row number
                'add_button'    => __( 'Add Another Video Sources', 'vidrev-ads' ),
                'remove_button' => __( 'Remove Video Sources', 'vidrev-ads' ),
                'sortable'      => true, // beta
            ),  
            'attributes' => array(
                    'required' => true, // Will be required only if visible.
                    'data-conditional-id' => $prefix . 'preroll_sources',
                    'data-conditional-value' => 'selfhosted',
                )       
        ) );

        // Id's for group's fields only need to be unique for the group. Prefix is not needed.
        $ads_box->add_group_field( $selfhosted_sources_group, array(
            'name' => __( 'Video URL', 'vidrev-ads' ),
            'id'   => $prefix.'selfhosted_url',
            'type' => 'file',
            'options' => array(
            'url' => true, // Hide the text input for the url
            'add_upload_file_text' => 'Add Video' // Change upload button text. Default: "Add or Upload File"
        ),
        ) );
fujianto commented 9 years ago

I found out on Group Field data-conditional-id and data-conditional-value doesn't exist on the markup.When I add it manually, the conditional is working. Any idea how to fix it?

jcchavezs commented 9 years ago

It looks like the group element does not consider the attributes you pass to. We need to add some changes here but first we need to see if the API allow us to do so.

jcchavezs commented 9 years ago

@fujianto, I think it will take some time. I will look into it during the weekend.

fujianto commented 9 years ago

Is it possible to filter render_group() output to add data-conditional-id and data-conditional value? Currently, I add it manualy on CMB2.php like this.

  $con_id = $field_group->args('attributes')['data-conditional-id']; 
  $con_value = $field_group->args('attributes')['data-conditional-value']; 

  echo '<div class="cmb-row cmb-repeat-group-wrap"><div class="cmb-td"><div id="', $field_group->id(), '_repeat"  data-conditional-id="'.$con_id.'" data-conditional-value="'.$con_value.'" class="cmb-nested cmb-field-list cmb-repeatable-group', $sortable, $repeat_class, '" style="width:100%;">';

Alright, I'll be waiting for it :+1:

EricTousignant commented 8 years ago

Hello, I needed this feature on a client's project and managed to fix this issue.

The problem is the data-conditional-id is not matching the expected field if it's inside a repeatable region. The CMB2 generated name attribute is in the form of an array with an iterator.

After some tests (forcing a specific data-conditional-id), I was able to make the plugin work with repeatable regions by doing as follow:

  1. Make 'data-conditional-id' => 'XXXXX[{#}][YYYYY]', where
    • XXXXX is the group's id
    • YYYYY the data-conditional-id.
  2. Modify the cmb2-conditionals.js file to check if the id has {#} in the string and if true, replace it with the current repeatable row (available in a data-iterator attribute).

Changes to cmb2-conditionals.js:14

if ( id.match('{#}') ) {
    id = id.replace('{#}', $e.closest('[data-iterator]').data('iterator') );
    $e.attr('data-conditional-id', id);
}

Like I said in another issue, I will try to do a pull request as soon as I'm done with this project.

jrfnl commented 8 years ago

I've been working on patching both the issues contained in this thread:

  1. Conditional groups (original issue from @fujianto ) - see https://github.com/jrfnl/cmb2-conditionals/tree/feature/fix-issue-2-group-conditionals
  2. Fields conditional on other fields within groups (issue from @EricTousignant ) - see https://github.com/jrfnl/cmb2-conditionals/tree/feature/conditionals-within-groups

For issue 1, a new filter needs to be added to CMB2 - see https://github.com/WebDevStudios/CMB2/pull/582. Once that filter is in, it's doable. To do: add save filtering for groups.

For issue 2, I've got the principle of it working, but am running into a js thingie which I don't have time to debug at the moment. For repeatable groups, the conditions are not working for newly added entries as the event listeners are not being added to the new entries. I'm pretty sure this is because the adding of the event listener is done within a function and not as a 'continuous listener for events', which means that - AFAICS - solving this would require major refactoring of the js file.

If anyone knows a quick/better solution for issue 2, please feel free to send a PR against the feature branch in my fork. Once the issue is solved, I'll PR against the main repo here.

jrfnl commented 8 years ago

Hi @EricTousignant Part 2 of this issue - the conditions between group fields - should be fixed by PR #17. Would be great if you would take the time to test it. I've used a slightly different syntax than the one you proposed. The example file in the PR contains an example of how I've implemented it.

EricTousignant commented 8 years ago

Hello @jrfnl, I did not have an opportunity to test your fix. It seems to have been rejected?

Please let me know if you want me to test something again?

It seems my fix is not flawless either (the javascript part fails to work on second item).

jrfnl commented 8 years ago

Hi @EricTousignant

It seems to have been rejected?

Not as far as I know, the PR is still open. (The red cross behind it has nothing to do with the PR, but with a wrongly configured Travis setup)

Please let me know if you want me to test something again?

If you could check out my branch and test with it, that would be great! The PR contains the complete changelog of fixes included and changes I made to make it work if you need more information.

It seems my fix is not flawless either (the javascript part fails to work on second item).

Yeah, I ran into that, which is why I completely refactored the js to work top-down instead of bottom-up. The end-result being that conditionals in newly added groups will continue to work.

Berdych commented 8 years ago

Hi @jrfnl I've just applied your branch and it solved my problem with newly added repeatable entries. Thank you very much. But I still need your help. How can I hide 'type' => 'title' in group from here? http://screenshotuploader.com/s/1604okk3n

jrfnl commented 8 years ago

@Berdych Thanks for testing & glad to hear it works for you!

How can I hide 'type' => 'title' in group from here?

AFAICS this is a CMB2 native question and has nothing to do with CMB2-conditionals or this issue.

Berdych commented 8 years ago

@jrfnl Your version of extension does not work in Options and Settings Pages - File: theme-options-cmb.php

I have added edit.php but it didn`t help.

if(!in_array($pagenow, array('post-new.php', 'post.php', 'edit.php'))) {

g4egu

jrfnl commented 8 years ago

@Berdych Andrew, just checking - but did the extension work in Options and Settings pages before applying the changes ? If not, this is not an issue with the PR, but with CMB2-conditionals as it was.

The PR only addresses the conditionals within groups and related issues found when working on that. Nothing else.

There have been several issues reported by other people about using CMB2-conditionals in Settings pages #18, #19, so I kind of think this is unrelated to my PR.

Would love to fix it (and several other things), but that will be so much easier once the PR is merged (otherwise there will be lots of conflicts), so I'm waiting on that.

Berdych commented 8 years ago

@jrfnl The simplest solution I could find to the Settings page: <div id="post"> <?php cmb2_metabox_form( $this->metabox_id, $this->key ); ?> </div>

jrfnl commented 8 years ago

@Berdych Yes, that would work ;-) Might be an idea to add an FAQ about this to the readme ?

Berdych commented 8 years ago

Your right. Add it. I think you can guess what the problem was. ;-)

siamzam commented 8 years ago

@jrfnl Thanks for the solution to the issue 2 "Fields conditional on other fields within groups". It works great. However, there is one small problem. Whenever the group elements are sorted using the up and down buttons the conditional functionality does not work e.g. all the hidden elements are shown again.

Can you plz have a look at that?

jrfnl commented 8 years ago

@siamzam When I have some time, I'll try and have a look. If you'd find a solution yourself in the mean time, you could send in a PR against the feature branch and I'll have a look.

jrfnl commented 8 years ago

@jcchavezs Could you please re-open this issue ? This has only partially been fixed.

Open :

  1. Conditional groups (original issue from @fujianto ) - see https://github.com/jrfnl/cmb2-conditionals/tree/feature/fix-issue-2-group-conditionals

A new filter needs to be added to CMB2 - see WebDevStudios/CMB2#582. Once that filter is in, it's doable. To do: add save filtering for groups.

jcchavezs commented 8 years ago

Sure @jrfnl

ShariqKhan2012 commented 8 years ago

Hi, Thank you for the wonderful add-on!

I have a question though. Has the code for making entire groups conditional been implemented? I ask because I am nto able to do so. May be I am using incorrect code?

My code is:

add_action( 'cmb2_admin_init', 'dummy_register_repeatable_group_field_metabox' );
/**
 * Hook in and add a metabox to demonstrate repeatable grouped fields
 */
function dummy_register_repeatable_group_field_metabox() {
    $prefix = 'dummy_';

    $dummy_settings_metabox  = new_cmb2_box( array(
        'id'             => $prefix . 'settings_metabox',
        'title'          => __( 'dummy Test Type Settings', 'cmb2' ),
        'object_types'   => array( 'dummy_test_type' ),
        'classes'    => 'myclass', // Extra cmb2-wrap classes
    ) );

    $feature_layout      = $dummy_settings_metabox->add_field( array(
        'name'   => __( 'Feature Layout', 'cmb2' ),
        'id'     => $prefix . 'feature_layout',
        'type'             => 'radio',
        'show_option_none' => false,
        'options'          => array(
            'inline'     => 'inline',
            'separate'   => 'separate',
        ),
        //'default'            => 'inline',
        'attributes'       => array(
            'required'       => 'required',
        ),
    ) );

    $dummy_plans_metabox     = new_cmb2_box( array(
        'id'             => $prefix . 'table_metabox',
        'title'          => __( 'Test Type', 'cmb2' ),
        'object_types'   => array( 'dummy_test_type' ),
        'classes'    => 'myclass', // Extra cmb2-wrap classes
    ) );
    // $group_field_id is the field id string, so in this case: $prefix . 'demo'
    $table_features_group    = $dummy_plans_metabox->add_field( array(
        'id'         => $prefix . 'features_group',
        'type'       => 'group',
        'sortable'       => false,
        'repeatable'     => false,
        'before_group'     => '<div class="before_group">',
        'after_group'      => '</div>',
        'before_group_row' => '<div class="before_group_row">',
        'after_group_row'  => '</div>',
        'display_cb' => 'cmbtest_display_plans', // Output the display of the column values through a callback.
        'classes'    => 'myclass2', // Extra cmb2-wrap classes
        'attributes' => array(
            'data-conditional-id'    => $prefix . 'feature_layout' , 
            'data-conditional-value' => 'inline',
        ),
    ) );
    $table_feature_tt        = $dummy_plans_metabox->add_group_field( $table_features_group, array(
        'name'   => __( 'FeaturesTT', 'cmb2' ),
        'id'     => 'features_tt',
        'type'             => 'radio',
        'show_option_none' => false,
        'options'          => array(
            'inline'     => 'inline',
            'separate'   => 'separate',
        ),
        //'default'            => 'inline',
        'attributes'       => array(
            'required'       => 'required',
        ),
    ) );

    $table_feature       = $dummy_plans_metabox->add_group_field( $table_features_group, array(
        'name'   => __( 'Features', 'cmb2' ),
        'id'     => $prefix . 'features',
        'type'   => 'text',
        //'repeatable' => 'true',
        'attributes' => array(
            'required'               => true, // Will be required only if visible.
        ),
    ) );
}

Best regards, Shariq Khan

kassanj commented 7 years ago

Hi there, has anybody been able to resolve hide/show group type? I would like the entire group to go away, but I'm still left with the image below. Any help would be greatly appreciated! screen shot 2016-10-21 at 3 55 16 pm

jimboobrien commented 7 years ago

This is still a major problem. When new elements are added to the page via a group add button it has no idea what should be conditionally shown and defaults to the original state before fields were shown/hidden before/after cloning.

I have found some code in the original CMB2.js file that when commented out fixes some of my issues: https://github.com/CMB2/CMB2/blob/trunk/js/cmb2.js#L558

Please tell me how I can contribute to try and help fix this, or if it is still in development.

jcchavezs commented 7 years ago

Hi @jimobrien, feel free to open a PR to address those issues, we will be more than happy to merge it.

On ons. 23. aug. 2017, 17.48 Jimobrien notifications@github.com wrote:

This is still a major problem. when new elements are added to the page via a group add button it has no idea what should be conditionally shown and defaults to the original state before fields were shown/hidden before/after cloning.

I have found some code in the original CMB2.js file that when commented out fixes some of my issues: https://github.com/CMB2/CMB2/blob/trunk/js/cmb2.js#L558

Please tell me how I can contribute to try and help fix this, or if it is still in development.

  • Jim

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/jcchavezs/cmb2-conditionals/issues/2#issuecomment-324378605, or mute the thread https://github.com/notifications/unsubscribe-auth/AC7sAptymrwOjmYOQQ4WuDUE4RR6uD4Pks5sbEnngaJpZM4FzB6X .

-- José Carlos Chávez Spain: (+34) 672921339 Perú: (+51) 708-5422 Anexo 918

https://github.com/jcchavezs

jimboobrien commented 7 years ago

@jcchavezs I found the line of code in your plugin that is the problem. It is in line 201 of cmb2-conditionals.js. The line above it (200) has a comment that says Hides all conditional elements, but the selector .cmb-row:first is only hiding the first field in the group.

For now I suggest commenting out line 201 because that fixes my issue, but that does not solve the problem. I am happy to submit a pull request to comment that line out, but I think we should work together to find a better solution to show/hide new elements added to the page in a repeatable group.

manzoorwanijk commented 6 years ago

It still doesn't work. I have a non-repeatable field group conditionally dependent upon a checkbox. I manually tried to add the group attributes but it still didn't work.