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

#states does not properly hide title and description of checkboxes element #711

Closed
jenlampton opened this issue Feb 13, 2015 · 6 comments
Closed

Comments

@jenlampton
Copy link
Member

I have the following form element:

$form['services']['enabled'] = array(
        '#type' => 'checkboxes',
        '#title' => t('Icons'),
        '#options' => $sorted_options,
        '#default_value' => (isset($settings['enabled'])) ? $settings['enabled'] : $defaults,
        '#description' => t('Please select the icons you would like displayed in this block.'),
        '#states' => array(
          'visible' => array(
            ':input[name="block_settings[services][display]"]' => array('value' => 'manual'),
          ),
        ),
      );

When the display target element is toggled, the checkboxes correctly appear and disappear, but the element title and description are always visible. It looks like the display:none is being added to the wrong div.

@bt-eric
Copy link

bt-eric commented Mar 21, 2015

For the visibility state, states.js looks for the nearest .form-item, .form-submit, or .form-wrapper element and applies the appropriate display style to it. For issue #197, the theme_checkboxes and theme_radios functions were removed in previous commits in favor of theme_container. This introduced a form-wrapper div around the form item value but not the form item label (there's no label in a container). This pull request modifies theme_container to not add the form-wrapper class if $element['#type'] exists and does not equal container.

@quicksketch
Copy link
Member

Thanks @bt-eric for researching this one and finding the root cause. I'm not sure about the solution proposed. Perhaps we could use different attribute besides #type on the element to determine if the class should be added.

Would something like this work?

if (empty($element['#attributes']['class'])) {
  $element['#attributes']['class'][] = 'form-wrapper';
} 

Then if checkboxes or radios add a different class, they won't end up with the form-wrapper class as well.

@quicksketch quicksketch added this to the 1.0.6 milestone Mar 22, 2015
@quicksketch
Copy link
Member

Let's try to fix this for the next bugfix release.

@bt-eric
Copy link

bt-eric commented Mar 28, 2015

I agree the check for an existing class is a better approach, and it works well. The pull request has been updated.

@quicksketch
Copy link
Member

Sweet. Thanks! I'll try giving this a manual go and see if it fixes the configuration display issues in Views, where this problem is most apparent.

@quicksketch
Copy link
Member

Confirmed this solves the problem in Views as well. It was apparent when using the "Content type" as an exposed filter, the list of content types to expose was a set of checkboxes whose title would never go away. This fixes the problem and the title is now hidden when the checkboxes are hidden. Nice one!

Merged this in as a squashed commit backdrop/backdrop@c499829 into 1.x and 1.0.x. Thanks @bt-eric!

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