Easy-digital-downloads: CSS Class Argument for Settings API Fields

Created on 2 Nov 2016  路  14Comments  路  Source: easydigitaldownloads/easy-digital-downloads

Handled in PR #5138

Currently if you need to add Class Names to a Field for either Field-Specific CSS or JavaScript you either have to rely on the ID of the Field or use some creative selectors.

If we were to have a class Argument for the Settings API, Class Name(s) could be defined so that more general Classes (Such as ones to control color, visibility, etc.) can be added to Fields for Styling or to target specific groups of Fields via JavaScript.

type-feature

All 14 comments

PR looks pretty good at first glance! Thanks!

@d4mation I'm testing in an extension of my own and while both strings and arrays work, it appears the class is also being added to the wrapping <tr> for the settings. While I believe that was unintentional, it's also worth noting that only strings are added (to the <tr>. Arrays output Array and throw a notice.

screenshot 2016-11-03 10 26 37

screenshot 2016-11-03 10 31 47

@SDavisMedia That's odd that it is adding to the <tr> at all. It must have something to do with how do_settings_section() is generating things. It only seems to be happening with the Checkbox Callback as far as I have seen. I'll check into this.

@d4mation I'm also seeing it with text and select callbacks. I think it's happening across the board.

@SDavisMedia Interesting. I didn't see it at all until I tried a Checkbox.

I found the cause though:

wp-admin/includes/template.php

function do_settings_fields($page, $section) {
    global $wp_settings_fields;

    if ( ! isset( $wp_settings_fields[$page][$section] ) )
        return;

    foreach ( (array) $wp_settings_fields[$page][$section] as $field ) {
        $class = '';

        if ( ! empty( $field['args']['class'] ) ) {
            $class = ' class="' . esc_attr( $field['args']['class'] ) . '"';
        }

        echo "<tr{$class}>";

        if ( ! empty( $field['args']['label_for'] ) ) {
            echo '<th scope="row"><label for="' . esc_attr( $field['args']['label_for'] ) . '">' . $field['title'] . '</label></th>';
        } else {
            echo '<th scope="row">' . $field['title'] . '</th>';
        }

        echo '<td>';
        call_user_func($field['callback'], $field['args']);
        echo '</td>';
        echo '</tr>';
    }
}

Changing the Argument Name to something different than class or otherwise ensuring that implode() is being called on it before getting passed to add_settings_field() should fix it.

Changing the Argument Name will ensure that it isn't added to both the Field and the <tr> which I think will give it more expected functionality.

I just changed the argument name to field_class for the PR. This appears to solve @SDavisMedia's problem while making the argument slightly more semantic as it only applies to the Field rather than to the full Row or to any other elements like the label.

Alright that's working perfectly for me now. I've tested adding a string with no spaces, a string with spaces, a string to an element that already has a class, and arrays to all. Everything checks out. :+1:

Thanks again, @d4mation!

@d4mation Let a comment on the PR. If that's something you have the time for awesome, if not I'll move it into it's own function, keeping your commit history in tact for credit. Nice work.

Taking a peek into it right now. Should be pretty simple to implement. Function name of edd_sanitize_html_class sound fine?

@cklosowski Just added a DRY function in there for sanitizing the HTML Class Names called edd_sanitize_html_class. Tested it locally and everything seems to work fine. As a result the only way to add multiple class names via $args['field_class'] is to use an Array as the spaces get stripped out of the String, but that shouldn't be a huge deal.

@d4mation I'm fine with multiple classes requiring an array. I think that's the preferred behavior honestly. It's the only way to make sure that classes are sanitized properly which we have to do.

@SDavisMedia Mind testing this one more time? Just had it updated to be a little less repetitive and sanitize. Looks like it's fine for me, but wanted to get a CSS Guru's perspective ;)

@cklosowski Looking good here still. Having an empty array or empty string as the field_class value outputs an empty class="" attribute if no class is already present. As long as that's not going to be an issue, all is well here.

As long as we always get class="" and never class we are fine. The first is still valid markup, where as class will fail validation.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

amdrew picture amdrew  路  5Comments

mindctrl picture mindctrl  路  4Comments

colomet picture colomet  路  4Comments

zackkatz picture zackkatz  路  4Comments

boluda picture boluda  路  4Comments